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

doc: fix code examples in url.md #13288

Closed
wants to merge 3 commits into from
Closed

doc: fix code examples in url.md #13288

wants to merge 3 commits into from

Conversation

vsemozhetbyt
Copy link
Contributor

Checklist
Affected core subsystem(s)

doc, url

  • Update outputs in code examples.
  • Refine spaces in code examples.
  • Restore missing part in code example (unify with other examples).

This is a retry of #13268 after #12710 landed (which has made url.md too different doc to just resolve the conflicts). Sorry for re-bothering.

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. url Issues and PRs related to the legacy built-in url module. labels May 29, 2017
@vsemozhetbyt
Copy link
Contributor Author

Copy link
Member

@watilde watilde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice :)

refack
refack approved these changes May 29, 2017
@@ -515,7 +516,7 @@ const params = new URLSearchParams({
query: ['first', 'second']
});
console.log(params.getAll('query'));
// Prints ['first,second']
// Prints [ 'first,second' ]
Copy link
Contributor

@refack refack May 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[ 'first', 'second' ] space after the comma

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an array with a single element :) It literally prints [ 'first,second' ].

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. I was trying console.log(['first','second'])

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the extensive structure changes messing with the other PR. This LGTM

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vsemozhetbyt
Copy link
Contributor Author

Landed in f1b2e68

@vsemozhetbyt vsemozhetbyt deleted the url.md-2 branch May 31, 2017 23:28
vsemozhetbyt added a commit that referenced this pull request May 31, 2017
* Update outputs.
* Refine spaces.
* Restore missing part.

PR-URL: #13288
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
jasnell pushed a commit that referenced this pull request Jun 5, 2017
* Update outputs.
* Refine spaces.
* Restore missing part.

PR-URL: #13288
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@jasnell jasnell mentioned this pull request Jun 5, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
@MylesBorins
Copy link
Contributor

This does not land cleanly in LTS. Please feel free to manually backport. Please also feel free to replace do-not-land if it is being backported

@vsemozhetbyt
Copy link
Contributor Author

Almost all the changes is for new URL format not applied for x6 docs, so do-not-land is valid here (other nits will be fixed when doc linting is backported).

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. url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants