Skip to content
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

docs: fs - remove encoding list and link to buffer #2796

Conversation

claudiorodriguez
Copy link
Contributor

Removes the accepted encoding lists from fs.createReadStream and fs.createWriteStream, linking instead to Buffer which has a list of accepted encodings. Would solve #2787

@Fishrock123 Fishrock123 added the doc Issues and PRs related to the documentations. label Sep 10, 2015
@Fishrock123
Copy link
Contributor

SGTM

@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Sep 10, 2015
@trevnorris
Copy link
Contributor

LGTM

@silverwind
Copy link
Contributor

@fansworld-claudio can you link to buffer.html#buffer_buffer? That anchor is closer to the list of encodings. Maybe also use a shared anchor (see the bottom of many markdown files) for both links.

@claudiorodriguez
Copy link
Contributor Author

@silverwind Good idea, change pushed.

@jasnell
Copy link
Member

jasnell commented Nov 5, 2015

@silverwind .. since you had the last comment to be address can you PTAL.
meanwhile, LGTM

@silverwind
Copy link
Contributor

LGTM

jasnell pushed a commit that referenced this pull request Nov 5, 2015
PR-URL: #2796
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Nov 5, 2015

Landed in 30ce3eb

@jasnell jasnell closed this Nov 5, 2015
rvagg pushed a commit that referenced this pull request Nov 7, 2015
PR-URL: #2796
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Nov 11, 2015
@MylesBorins
Copy link
Contributor

landed in lts-v4.x-staging as 5689630

MylesBorins pushed a commit that referenced this pull request Nov 16, 2015
PR-URL: #2796
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this pull request Dec 4, 2015
PR-URL: #2796
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell jasnell mentioned this pull request Dec 17, 2015
jasnell pushed a commit that referenced this pull request Dec 17, 2015
PR-URL: #2796
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Dec 23, 2015
PR-URL: #2796
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
@rvagg
Copy link
Member

rvagg commented Jan 15, 2016

@thealphanerd you only landed one of these commits, the other, 87c6c77, is not on v4.x but it doesn't land cleanly.

I'd suggest either leaving that one off v4.x or, @fansworld-claudio if you'd like to submit a PR for that commit against v4.x-staging then we can land it.

@jasnell
Copy link
Member

jasnell commented Jan 15, 2016

@rvagg ... if I recall correctly, the commit that actually landed in master was squashed into a single commit (5689630). It looks like @thealphanerd picked that commit into v4.x-staging which is why the second commit here doesn't appear and doesn't land clean. (I could be wrong tho)

@rvagg
Copy link
Member

rvagg commented Jan 16, 2016

$ git log master --format='%h %an: %s' | grep fansworld-claudio
2a29b70 fansworld-claudio: doc: url.format - true slash postfix behaviour
9a628e2 fansworld-claudio: test: module loading error fix solaris #3798
30ce3eb fansworld-claudio: docs: fs - change links to buffer encoding to Buffer class anchor
2e38079 fansworld-claudio: docs: fs - remove encoding list and link to buffer
49f965c fansworld-claudio: docs: add missing shell option to execSync

30ce3eb and 2e38079

They show up in branch-diff, when comparing v4.x-staging to master:

  • [2e38079ea4] - docs: fs - remove encoding list and link to buffer (fansworld-claudio)

And v4.x-staging to v5.x:

  • [7a84fa6c60] - docs: fs - remove encoding list and link to buffer (fansworld-claudio)

@MylesBorins
Copy link
Contributor

@rvagg looks like we are now stuck with 7a84fa6 forever showing up as a false positive when doing audits... perhaps there is a way to add meta data to the comments inside the commit that branch-diff / changelog maker can use?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants