doc: change e.g to for example by sreepurnajasti · Pull Request #18397 · nodejs/node

sreepurnajasti · 2018-01-26T16:32:22Z

Checklist

documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

doc

Trott · 2018-01-26T19:44:40Z

CPP_STYLE_GUIDE.md

 ## Ownership and Smart Pointers

-"Smart" pointers are classes that act like pointers, e.g.
+"Smart" pointers are classes that act like pointers, for example:


The : is fine for the parenthetical stuff but it's a problem in ordinary prose like this. I don't think we want to do a global find/replace on this. We want to evaluate each item independently. For example, this instance should probably be changed to such as rather than for example:.

Nit: Get rid of the scare quotes in this sentence and instead italicize the term being defined:

"Smart" pointers -> _Smart pointers_

@Trott Done. Please review.

Trott · 2018-01-26T19:45:59Z

doc/api/debugger.md

 V8 Inspector can be enabled by passing the `--inspect` flag when starting a
-Node.js application. It is also possible to supply a custom port with that flag,
-e.g. `--inspect=9222` will accept DevTools connections on port 9222.
+Node.js application. It is also possible to supply a custom port with that flag,for example: `--inspect=9222` will accept DevTools connections on port 9222.


Please get rid of the comma and put the the whole example in parentheses.

@Trott Thanks. Done!!

Trott

Left some change suggestions I'd like to see addressed before this lands. So I'm going to make it explicit with Request changes. Thanks!

fhinkel · 2018-01-29T13:02:49Z

I think the : is disrupting the flow and incorrect. I'm not sure what the benefit of this PR is, but I agree, that e.g definitely needs to be changed to either e.g., or for example.

sreepurnajasti · 2018-01-30T06:26:07Z

@fhinkel @tniessen Using e.g. or e.g., will always result in confusion(i.e which to use). So, replacing this instances with for example could be a good move. Please suggest if : is incorrect. One alternative, I could think of is ",". Please, let me know your opinion.

fhinkel · 2018-01-30T08:50:23Z

Sorry, there's no confusion in American English, it's e.g., and i.e.,.

BridgeAR

The line length is often above 80 characters.

sreepurnajasti · 2018-02-01T06:27:55Z

@fhinkel @Trott Thanks for the explanation. Please guide me if I need to make any changes/ close in this pr.

sreepurnajasti · 2018-02-01T11:05:51Z

@BridgeAR Thanks for pointing out but I am unable to figure out which line is that. Please help.

BridgeAR · 2018-02-01T11:22:04Z

@sreepurnajasti I am not sure what IDE you use but while for example using Visual Studio Code you can install a extension that you can use for to either highlighting it or to change it right away.

This should also be possible with different IDEs. Other than that you can also just have a look at the changed lines here on github and every time the line seems longer than the other ones surrounding that line, it is probably above 80 characters.

sreepurnajasti · 2018-02-01T11:31:18Z

@BridgeAR Got it. I will make changes as soon as I get a decision on this pr. Thanks!!

BridgeAR · 2018-02-06T22:07:35Z

Ping @Trott

Trott · 2018-02-07T06:03:41Z

Ping @Trott

I'm still -1. This should not be done with a find/replace. Each instance needs to be examined and handled thoughtfully. The : in particular is questionable in most of these cases.

Trott · 2018-02-07T06:04:16Z

The : in particular is questionable in most of these cases.

Each case should be examined and a determination made as to whether there should be a comma or not, and whether or not for example is the right text to use.

Trott · 2018-02-07T06:06:21Z

BUILDING.md

 *Note*: On Windows, running Node.js in windows terminal emulators like `mintty`
  requires the usage of [winpty](https://github.com/rprichard/winpty) for
-  Node's tty channels to work correctly (e.g. `winpty node.exe script.js`).
+  Node's tty channels to work correctly (for example: `winpty node.exe script.js`).


Nit: while in here, maybe Node's -> Node.js's or even just remove Node's altogether.

Trott · 2018-02-07T06:07:07Z

BUILDING.md

 ```

-### Android/Android-based devices (e.g. Firefox OS)
+### Android/Android-based devices (for example: Firefox OS)


This would probably be better/clearer as (Firefox OS, etc.) rather than (for example: Firefox OS).

Trott · 2018-02-07T06:08:14Z

COLLABORATOR_GUIDE.md

 * changing the side effects of using a particular API

-Purely additive changes (e.g. adding new events to `EventEmitter`
+Purely additive changes (for example: adding new events to `EventEmitter`


This is a place where such as is likely called for rather than for example.

Trott · 2018-02-07T06:08:26Z

COLLABORATOR_GUIDE.md


 Note that errors thrown, along with behaviors and APIs implemented by
-dependencies of Node.js (e.g. those originating from V8) are generally not
+dependencies of Node.js (for example: those originating from V8) are generally not


Trott · 2018-02-07T06:08:41Z

COLLABORATOR_GUIDE.md

 * Breaking changes should *never* land in Current or LTS except when:
  * Resolving critical security issues.
-  * Fixing a critical bug (e.g. fixing a memory leak) requires a breaking
+  * Fixing a critical bug (for example: fixing a memory leak) requires a breaking


comma instead of colon

Trott · 2018-02-07T06:09:09Z

COLLABORATOR_GUIDE.md

 hint: not have locally. This is usually caused by another repository pushing
 hint: to the same ref. You may want to first integrate the remote changes
-hint: (e.g. 'git pull ...') before pushing again.
+hint: (for example: 'git pull ...') before pushing again.


This is output from a command. It should not be changed.

Trott · 2018-02-07T06:11:04Z

I started to go through and leave comments but stopped well before reviewing all the changes. Hopefully you get the idea. This may be more manageable as separate pull requests for each file or something like that.

fhinkel · 2018-02-07T12:00:01Z

I agree with @Trott. This is not a mechanical change and the review cannot be handled with this many files affected. My suggestion would be to make small PRs that improve the language. The e.g., language does not need to be consistent in all the documents. It's fine if documentations is improved iteratively and if they're minor style differences at some point in time.

I'd suggest making one PR, once that is landed, use the feedback and learnings for the next PR. Otherwise reviewers get overwhelmed with a large number of PRs. Thanks!

BridgeAR · 2018-02-10T20:16:13Z

Due to the mentioned concerns I am going to close this. @sreepurnajasti thanks a lot for your contribution nevertheless and please go ahead as suggested and open a smaller PR. In that case it can probably also land much faster in general.

sreepurnajasti added 4 commits January 26, 2018 14:01

doc: change e.g. to for example

5021d43

doc: change e.g to for example in api docs

bf93260

doc: change e.g to for example in docs

15e8963

doc: change e.g to for example in guides doc

fb24a5d

nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. labels Jan 26, 2018

jasnell approved these changes Jan 26, 2018

View reviewed changes

Trott reviewed Jan 26, 2018

View reviewed changes

Trott requested changes Jan 26, 2018

View reviewed changes

maclover7 force-pushed the master branch from bb5575a to 993b716 Compare January 26, 2018 22:03

cjihrig force-pushed the master branch from 993b716 to 082f952 Compare January 26, 2018 22:36

doc: minor changes in docs

bcef244

BridgeAR reviewed Jan 31, 2018

View reviewed changes

Trott reviewed Feb 7, 2018

View reviewed changes

BridgeAR closed this Feb 10, 2018

Uh oh!

Conversation

sreepurnajasti commented Jan 26, 2018

Checklist

Affected core subsystem(s)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Trott left a comment

Choose a reason for hiding this comment

Uh oh!

fhinkel commented Jan 29, 2018

Uh oh!

sreepurnajasti commented Jan 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fhinkel commented Jan 30, 2018

Uh oh!

BridgeAR left a comment

Choose a reason for hiding this comment

Uh oh!

sreepurnajasti commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sreepurnajasti commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BridgeAR commented Feb 1, 2018

Uh oh!

sreepurnajasti commented Feb 1, 2018

Uh oh!

BridgeAR commented Feb 6, 2018

Uh oh!

Trott commented Feb 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Trott commented Feb 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Trott commented Feb 7, 2018

Uh oh!

fhinkel commented Feb 7, 2018

Uh oh!

BridgeAR commented Feb 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sreepurnajasti commented Jan 30, 2018 •

edited

Loading

sreepurnajasti commented Feb 1, 2018 •

edited

Loading

sreepurnajasti commented Feb 1, 2018 •

edited

Loading

Trott commented Feb 7, 2018 •

edited

Loading