-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DOC] Enhanced RDoc for Net::HTTP #77
Conversation
BurdetteLamar
commented
Nov 15, 2022
- Revises most of class doc for Net::HTTP. (Does not modify sections "Following Redirection" and after.)
- Treats a few methods, including Net::HTTP.start.
- Adds to class docs for Net::HTTPRequest and Net::HTTPResponse.
lib/net/http.rb
Outdated
# connections. They are not recommended if you are performing many HTTP | ||
# requests. | ||
# - https://jsonplaceholder.typicode.com. | ||
# - http:example.com. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# - http:example.com. | |
# - http://example.com. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed (3 places). Should "About the Examples" be pushed out to an includable file (in a future PR)? It's now used in three places, and may be in more.
Wrt headers, they have no apparent effect on either host we've mentioned. Would it be useful to introduce another "real" host (github rest api, for example) to have more "real" examples? (But not in this PR.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like https://httpbin.org/ that is more of a "swiss army knife" of endpoints to test various things would be useful (to demo things like headers, authentication, status codes, redirect, etc.).
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
I'll look into that. (But not for this PR?) |
A reader pointed out (offline) that I'd added to request.rb a new section for each of the 15 subclasses, but with text and examples for only three. For some (perhaps most) of the requests, the example host will not behave well; so I have removed the new sections altogether. |
# - \Net::HTTP::Get | ||
# - \Net::HTTP::Head | ||
# - \Net::HTTP::Post | ||
# - \Net::HTTP::Delete | ||
# - \Net::HTTP::Options | ||
# - \Net::HTTP::Trace | ||
# - \Net::HTTP::Patch | ||
# - \Net::HTTP::Put | ||
# - \Net::HTTP::Copy | ||
# - \Net::HTTP::Lock | ||
# - \Net::HTTP::Mkcol | ||
# - \Net::HTTP::Move | ||
# - \Net::HTTP::Propfind | ||
# - \Net::HTTP::Proppatch | ||
# - \Net::HTTP::Unlock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove the escaping so that it links to the subclasses?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The subclasses' docs are empty(ish).
# - \Net::HTTP::Get | ||
# - \Net::HTTP::Head | ||
# - \Net::HTTP::Post | ||
# - \Net::HTTP::Delete | ||
# - \Net::HTTP::Options | ||
# - \Net::HTTP::Trace | ||
# - \Net::HTTP::Patch | ||
# - \Net::HTTP::Put | ||
# - \Net::HTTP::Copy | ||
# - \Net::HTTP::Lock | ||
# - \Net::HTTP::Mkcol | ||
# - \Net::HTTP::Move | ||
# - \Net::HTTP::Propfind | ||
# - \Net::HTTP::Proppatch | ||
# - \Net::HTTP::Unlock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this list sorted? Should we sort it alphabetically (or some other way) so it's more intuitive to read?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're in the order as given in the Wikipedia docs we've linked (which I assumed is the same as in the underlying specs):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, that makes sense. I wonder how Wikipedia orders their list 🤔