-
-
Notifications
You must be signed in to change notification settings - Fork 33.5k
doc: general improvements to querystring.md copy #7023
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
Conversation
doc/api/querystring.md
Outdated
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'm wondering if anyone ever actually uses these sep
and eq
parameters :)
LGTM |
doc/api/querystring.md
Outdated
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.
s/key+value/key and value/
Nits addressed |
doc/api/querystring.md
Outdated
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.
FWIW I'm not sure the optional indicators here accurately reflect how the actual implementation does argument checking.
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.
Yeah, I was looking at that also. I need to go back in and verify this.
Updated |
@mscdex ... ping ... I'm still going to go back and check the args against the actual args checking but I'd like to get this PR landed and if there are changes necessary I'll do those separately. Does this LGTY? @nodejs/documentation |
@jasnell except for the args issues, LGTM |
0e7fcb4
to
b9e5e68
Compare
@mscdex ... I corrected the arg issues. Getting this landed! |
PR-URL: #7023 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Brian White <[email protected]>
Landed in 80f1fbb |
PR-URL: #7023 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Brian White <[email protected]>
Checklist
Affected core subsystem(s)
doc (querystring)
Description of change
General improvements to querystring.md copy
@nodejs/documentation @mscdex