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.

Sign up for GitHub

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

Jump to bottom

doc: add single arg scenario for util.format #12374

Closed
wants to merge 1 commit into from
Closed

doc: add single arg scenario for util.format #12374

wants to merge 1 commit into from

Conversation

tarunbatra
Copy link
Contributor

Set the expected outcome of util.format('%%') to be %%
instead of %.

Fixes: #12362

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. util Issues and PRs related to the built-in util module. labels Apr 12, 2017
@tarunbatra tarunbatra mentioned this pull request Apr 12, 2017
Closed
@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Apr 12, 2017 •
edited
Loading

Thank you for your contribution! Some nits:

  1. According to style guide, we usually wrap doc lines at 80 characters.
  2. In commit messages, we use full GitHub URLs with 'Fixes;' etc.

Trott
Trott approved these changes Apr 12, 2017
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

Doc change LGTM. Wrapping at 80 chars would be nice, although I wouldn't be surprised if there were other lines in the file that exceed 80 chars already...

@tarunbatra
Copy link
Contributor Author

@vsemozhetbyt @Trott thanx for pointing that out. I'll make the changes right away.

@tarunbatra
Copy link
Contributor Author

Done.

evanlucas
evanlucas suggested changes Apr 12, 2017
View reviewed changes
doc/api/util.md Outdated
without any formatting.

```js
util.format('%% %s') // '%% %s'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you include a semicolon here please? We try to follow the same coding style as the actual codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Changed that.

@tarunbatra
doc: add single arg scenario for util.format
636302f 
Set the expected outcome of `util.format('%%')` to be `%%`
instead of `%`.

Fixes: #12362
aqrln
aqrln approved these changes Apr 12, 2017
View reviewed changes
Copy link
Contributor

@aqrln aqrln left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

evanlucas
evanlucas approved these changes Apr 12, 2017
View reviewed changes
Copy link
Contributor

@evanlucas evanlucas left a comment

Choose a reason for hiding this comment

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

Thanks!

@benjamingr
Copy link
Member

We include the pull request URL under PR-URL in the commit message. I have done this for you.

I've landed this in 78af0df - thanks for your contribution to Node :)

@benjamingr benjamingr closed this Apr 13, 2017
benjamingr pushed a commit that referenced this pull request Apr 13, 2017
@tarunbatra
doc: add single arg scenario for util.format
78af0df 
Set the expected outcome of `util.format('%%')` to be `%%`
instead of `%`.

PR-URL: #12374
Fixes: #12362
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
evanlucas pushed a commit that referenced this pull request Apr 25, 2017
@tarunbatra @evanlucas
doc: add single arg scenario for util.format
7dc327b 
Set the expected outcome of `util.format('%%')` to be `%%`
instead of `%`.

PR-URL: #12374
Fixes: #12362
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@evanlucas evanlucas mentioned this pull request May 1, 2017
Merged
evanlucas pushed a commit that referenced this pull request May 1, 2017
@tarunbatra @evanlucas
doc: add single arg scenario for util.format
526fbcd 
Set the expected outcome of `util.format('%%')` to be `%%`
instead of `%`.

PR-URL: #12374
Fixes: #12362
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
evanlucas pushed a commit that referenced this pull request May 2, 2017
@tarunbatra @evanlucas
doc: add single arg scenario for util.format
ed13016 
Set the expected outcome of `util.format('%%')` to be `%%`
instead of `%`.

PR-URL: #12374
Fixes: #12362
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 15, 2017
@tarunbatra @MylesBorins
doc: add single arg scenario for util.format
2fc806e 
Set the expected outcome of `util.format('%%')` to be `%%`
instead of `%`.

PR-URL: #12374
Fixes: #12362
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 18, 2017
@tarunbatra @MylesBorins
doc: add single arg scenario for util.format
3ae8d00 
Set the expected outcome of `util.format('%%')` to be `%%`
instead of `%`.

PR-URL: #12374
Fixes: #12362
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request May 23, 2017
Merged
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
@tarunbatra @andrew749
doc: add single arg scenario for util.format
8fd5c73 
Set the expected outcome of `util.format('%%')` to be `%%`
instead of `%`.

PR-URL: nodejs/node#12374
Fixes: nodejs/node#12362
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack refack approved these changes

@evanlucas evanlucas evanlucas approved these changes

@Trott Trott Trott approved these changes

@lpinca lpinca lpinca approved these changes

@cjihrig cjihrig cjihrig approved these changes

@aqrln aqrln aqrln approved these changes

@vsemozhetbyt vsemozhetbyt vsemozhetbyt approved these changes

Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. util Issues and PRs related to the built-in util module.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

doc: confusing language in util.format() docs