-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: clarify allowed encoding parameter types #24230
Conversation
I think keeping the 'output' prefix is helpful in making it more clear that it's about what the function produces and not something to do with the contents of |
These links need fixing with this changes: |
@mscdex so you suggest |
@sam-github I'm fine with either one |
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.
LGTM % naming nit & anchor fixing
cbe9b37
to
7380867
Compare
@vsemozhetbyt How did you find those link errors? The markdown linter isn't find them :-(. PTAL, I think I addressed all comments. |
7380867
to
aef6001
Compare
I usually just grep old anchors in the repository for such signature changes) |
This fixes the incorrect enumerations of their possible values, which weren't up to date with the values actually supported. Also renamed two arguments that used "format" when they meant "encoding".
aef6001
to
1317f15
Compare
@vsemozhetbyt is a travis CI build sufficient for doc PRs, or do I need a full jenkins CI? |
Sorry, I just know that CI-lite is sufficient, i.e. linux-one + linter. I am not very aware of our different CI flavors. |
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.
Format LGTM)
ci-lite is what I was looking for, thanks. |
Landed in 426ca08 |
This fixes the incorrect enumerations of their possible values, which weren't up to date with the values actually supported. Also renamed two arguments that used "format" when they meant "encoding". PR-URL: nodejs#24230 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
This fixes the incorrect enumerations of their possible values, which weren't up to date with the values actually supported. Also renamed two arguments that used "format" when they meant "encoding". PR-URL: #24230 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
This fixes the incorrect enumerations of their possible values, which weren't up to date with the values actually supported. Also renamed two arguments that used "format" when they meant "encoding". PR-URL: nodejs#24230 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
This fixes the incorrect enumerations of their possible values, which weren't up to date with the values actually supported. Also renamed two arguments that used "format" when they meant "encoding". PR-URL: #24230 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
The code calls them encoding (mostly), and they are the encoding types
supported by Buffer, so call them that. Also fix the incorrect
enumerations of their possible values, which weren't up to date with the
values actually supported.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes