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: replace uses of `you` and other style nits #12673

Closed
wants to merge 2 commits into from

Conversation

@jasnell
Copy link
Member

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

doc/api/cluster.md Outdated
@@ -713,8 +713,8 @@ added: v0.11.2

The scheduling policy, either `cluster.SCHED_RR` for round-robin or
`cluster.SCHED_NONE` to leave it to the operating system. This is a
global setting and effectively frozen once you spawn the first worker
or call `cluster.setupMaster()`, whatever comes first.
global setting and effectively frozen once wither the first worker is spawned,

This comment has been minimized.

Copy link
@vsemozhetbyt

vsemozhetbyt Apr 26, 2017

Contributor

wither -> either?

This comment has been minimized.

Copy link
@jasnell

jasnell Apr 26, 2017

Author Member

heh... yes, is should be either

@jasnell jasnell force-pushed the jasnell:i-banish-you-from-the-docs branch Apr 26, 2017

doc/api/globals.md Outdated
In browsers, the top-level scope is the global scope. This means that
`var something`, within the browser, will define a new global variable. In
Node.js this is different. The top-level scope is not the global scope;
`var something` inside an Node.js module will be local to that module.

This comment has been minimized.

Copy link
@vsemozhetbyt

vsemozhetbyt Apr 26, 2017

Contributor

an Node.js -> a Node.js?

doc/api/modules.md Outdated
may itself have dependencies, and in some cases, these dependencies may even
collide or form cycles.
Packages can depend on one another. In order to install package `foo`, it
may be necessary to install a specific version of package `bar`. The `bar`

This comment has been minimized.

Copy link
@vsemozhetbyt

vsemozhetbyt Apr 26, 2017

Contributor

Two spaces in succession.

@lpinca
lpinca approved these changes Apr 27, 2017
@gibfahn
Copy link
Member

left a comment

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

doc/api/cluster.md Outdated
responsibility to manage the worker pool for your application's needs.
will be dropped and new connections will be refused. Node.js does not
automatically manage the number of workers, however. It is the application's
responsibility to manage the worker pool based on it's own needs.

This comment has been minimized.

Copy link
@gibfahn

gibfahn Apr 28, 2017

Member

it's -> its

doc/api/cluster.md Outdated
created. It is disconnected after the `'disconnect'` event is emitted.
This function returns `true` if the worker is connected to its master via its
IPC channel, `false` otherwise. A worker is connected to its master after it's
been created. It is disconnected after the `'disconnect'` event is emitted.

This comment has been minimized.

Copy link
@gibfahn

gibfahn Apr 28, 2017

Member

it's been -> it has been

The boolean `worker.exitedAfterDisconnect` lets you distinguish between voluntary
and accidental exit, the master may choose not to respawn a worker based on
this value.
The boolean `worker.exitedAfterDisconnect` allows distinguishing between

This comment has been minimized.

Copy link
@gibfahn

gibfahn Apr 28, 2017

Member

allows distinguishs -> is used to distinguish

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

doc/api/cluster.md Outdated
@@ -469,7 +467,7 @@ An alias to [`worker.exitedAfterDisconnect`][].

Set by calling `.kill()` or `.disconnect()`. Until then, it is `undefined`.

The boolean `worker.suicide` lets you distinguish between voluntary
The boolean `worker.suicide` allows distinguishs between voluntary

This comment has been minimized.

Copy link
@gibfahn

gibfahn Apr 28, 2017

Member

allows distinguishs -> is used to distinguish

as above...

doc/api/cluster.md Outdated
After calling `listen()` from a worker, when the `'listening'` event is emitted on
the server, a `'listening'` event will also be emitted on `cluster` in the master.
After calling `listen()` from a worker, when the `'listening'` event is emitted
on the server, a `'listening'` event will also be emitted on `cluster` in the

This comment has been minimized.

Copy link
@gibfahn

gibfahn Apr 28, 2017

Member

server, a -> server a

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

doc/api/modules.md Outdated
@@ -203,8 +200,8 @@ executed multiple times. This is an important feature. With it,
"partially done" objects can be returned, thus allowing transitive
dependencies to be loaded even when they would cause cycles.

If you want to have a module execute code multiple times, then export a
function, and call that function.
To have a module execute code multiple times, then export a function, and call

This comment has been minimized.

Copy link
@gibfahn
doc/api/modules.md Outdated
@@ -297,8 +294,8 @@ a done
in main, a.done=true, b.done=true
```

If you have cyclic module dependencies in your program, make sure to
plan accordingly.
Careful planning is required to allow cyclic module dependencies within an

This comment has been minimized.

Copy link
@gibfahn

gibfahn Apr 28, 2017

Member

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
These are mostly for historic reasons.

*Note*: It is strongly encouraged to place dependencies locally in
`node_modules` folders. These will be loaded faster, and more reliably.

This comment has been minimized.

Copy link
@gibfahn

gibfahn Apr 28, 2017

Member

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

doc/api/repl.md Outdated
@@ -499,9 +499,9 @@ by the `NODE_REPL_HISTORY` variable, as documented in the

For advanced line-editors, start Node.js with the environmental variable
`NODE_NO_READLINE=1`. This will start the main and debugger REPL in canonical
terminal settings which will allow you to use with `rlwrap`.
terminal settings which will allow use with `rlwrap`.

This comment has been minimized.

Copy link
@gibfahn

gibfahn Apr 28, 2017

Member

Maybe settings which -> settings, which

doc/api/net.md Outdated
@@ -650,7 +650,7 @@ added: v0.9.6
-->

The string representation of the local IP address the remote client is
connecting on. For example, if you are listening on `'0.0.0.0'` and the
connecting on. For example, when listening on `'0.0.0.0'` and the

This comment has been minimized.

Copy link
@gibfahn

gibfahn Apr 28, 2017

Member

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 Apr 26, 2017
doc: replace uses of `you` and other style nits
Replace uses of the pronouns `you` and `your` throughout
the docs + other minor style nits

@jasnell jasnell force-pushed the jasnell:i-banish-you-from-the-docs branch to 83acb55 Apr 28, 2017

@jasnell

This comment has been minimized.

Copy link
Member Author

commented Apr 28, 2017

@gibfahn ... updated!

@gibfahn
Copy link
Member

left a comment

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
doc: replace uses of `you` and other style nits
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

This comment has been minimized.

Copy link
Member Author

commented Apr 28, 2017

Landed in 71f22c8

@jasnell jasnell closed this Apr 28, 2017

@jasnell jasnell referenced this pull request May 11, 2017
@gibfahn gibfahn referenced this pull request Jun 15, 2017
2 of 3 tasks complete
@gibfahn

This comment has been minimized.

Copy link
Member

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
Projects
None yet
7 participants
You can’t perform that action at this time.