Skip to content

doc: replace uses of you and other style nits #12673

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
Closed
jasnell wants to merge 2 commits into from
Closed

doc: replace uses of you and other style nits #12673

jasnell wants to merge 2 commits into from

Conversation

@jasnell
Copy link
Member

@jasnell jasnell commented Apr 26, 2017

Replace uses of the pronouns you and your throughout the docs + other minor style nits

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 the doc Issues and PRs related to the documentations. label Apr 26, 2017
vsemozhetbyt
vsemozhetbyt reviewed Apr 26, 2017
View reviewed changes
doc/api/cluster.md Outdated
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Apr 26, 2017

Choose a reason for hiding this comment

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

wither -> either?

Copy link
Member Author

@jasnell jasnell Apr 26, 2017

Choose a reason for hiding this comment

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

heh... yes, is should be either

@jasnell jasnell force-pushed the i-banish-you-from-the-docs branch from d801879 to 3213cfd Compare April 26, 2017 17:38
vsemozhetbyt
vsemozhetbyt reviewed Apr 26, 2017
View reviewed changes
doc/api/globals.md Outdated
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Apr 26, 2017

Choose a reason for hiding this comment

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

an Node.js -> a Node.js?

vsemozhetbyt
vsemozhetbyt reviewed Apr 26, 2017
View reviewed changes
doc/api/modules.md Outdated
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Apr 26, 2017

Choose a reason for hiding this comment

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

Two spaces in succession.

gibfahn
gibfahn suggested changes Apr 28, 2017
View reviewed changes
Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

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

Some nits (most of them from the existing docs, but while you're there)

doc/api/cluster.md Outdated
Copy link
Member

@gibfahn gibfahn Apr 28, 2017

Choose a reason for hiding this comment

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

it's -> its

doc/api/cluster.md Outdated
Copy link
Member

@gibfahn gibfahn Apr 28, 2017

Choose a reason for hiding this comment

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

it's been -> it has been

doc/api/cluster.md Outdated
Copy link
Member

@gibfahn gibfahn Apr 28, 2017

Choose a reason for hiding this comment

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

allows distinguishs -> is used to distinguish

allows suggests that's just one of the things it can do right?

doc/api/cluster.md Outdated
Copy link
Member

@gibfahn gibfahn Apr 28, 2017

Choose a reason for hiding this comment

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

allows distinguishs -> is used to distinguish

as above...

doc/api/cluster.md Outdated
Copy link
Member

@gibfahn gibfahn Apr 28, 2017

Choose a reason for hiding this comment

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

server, a -> server a

I think that's easier to parse, happy to be disagreed with though.

doc/api/modules.md Outdated
Copy link
Member

@gibfahn gibfahn Apr 28, 2017

Choose a reason for hiding this comment

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

then

doc/api/modules.md Outdated
Copy link
Member

@gibfahn gibfahn Apr 28, 2017

Choose a reason for hiding this comment

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

to allow cyclic module dependencies to what? The sentence as is suggests that they won't be allowed unless you plan carefully, but AIUI what will happen is that the application just won't work correctly.

Maybe:

to allow cyclic module dependencies to work correctly within an application.

doc/api/modules.md Outdated
Copy link
Member

@gibfahn gibfahn Apr 28, 2017

Choose a reason for hiding this comment

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

Maybe locally in node_modules folders -> in the local node_modules folder

doc/api/repl.md Outdated
Copy link
Member

@gibfahn gibfahn Apr 28, 2017

Choose a reason for hiding this comment

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

Maybe settings which -> settings, which

doc/api/net.md Outdated
Copy link
Member

@gibfahn gibfahn Apr 28, 2017

Choose a reason for hiding this comment

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

I don't think when...and works here. Can we be more specific about what is listening on '0.0.0.0'?

Sorry for not giving a concrete suggestion, I can't think of a good way to say this.

jasnell added 2 commits April 28, 2017 05:32
@jasnell
doc: replace uses of you and other style nits
ea3efc6 
Replace uses of the pronouns `you` and `your` throughout
the docs + other minor style nits
@jasnell
Address feedback
83acb55
@jasnell jasnell force-pushed the i-banish-you-from-the-docs branch from 3213cfd to 83acb55 Compare April 28, 2017 12:45
@jasnell
Copy link
Member Author

jasnell commented Apr 28, 2017

@gibfahn ... updated!

gibfahn
gibfahn approved these changes Apr 28, 2017
View reviewed changes
Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM. Especially impressed by how you fixed the bits where I basically said "this is bad, I have no constructive suggestions, please make better..."

jasnell added a commit that referenced this pull request Apr 28, 2017
@jasnell
doc: replace uses of you and other style nits
71f22c8 
Replace uses of the pronouns `you` and `your` throughout
the docs + other minor style nits

PR-URL: #12673
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Apr 28, 2017

Landed in 71f22c8

@jasnell jasnell closed this Apr 28, 2017
@jasnell jasnell mentioned this pull request May 11, 2017
Closed
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
@gibfahn
Copy link
Member

gibfahn commented Jun 18, 2017

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

This looks reasonably easy to backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benjamingr benjamingr benjamingr approved these changes

@lpinca lpinca lpinca approved these changes

@cjihrig cjihrig cjihrig approved these changes

+2 more reviewers

@vsemozhetbyt vsemozhetbyt vsemozhetbyt approved these changes

@gibfahn gibfahn gibfahn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jasnell @gibfahn @benjamingr @lpinca @cjihrig @vsemozhetbyt @nodejs-github-bot