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: remove 'you' from writing-tests.md #13319

Closed
wants to merge 2 commits into from
Closed

Conversation

mhdawson
Copy link
Member

Noticed this while reading through writing-tests.md today.
As per style guide avoid the use of you, your etc.
Rational as per: http://www2.ivcc.edu/rambo/tip_formal_writing_voice.htm

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 May 30, 2017
@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented May 30, 2017

The commit and PR titles are a bit scary :) Maybe remove 'you'...?

@vsemozhetbyt vsemozhetbyt added the test Issues and PRs related to the tests. label May 30, 2017
@gibfahn
Copy link
Member

gibfahn commented May 30, 2017

It's quite amusing that http://www2.ivcc.edu/rambo/tip_formal_writing_voice.htm uses "you" the whole way through.

@@ -232,8 +232,8 @@ For performance considerations, we only use a selected subset of ES.Next
features in JavaScript code in the `lib` directory. However, when writing
tests, for the ease of backporting, it is encouraged to use those ES.Next
features that can be used directly without a flag in [all maintained branches]
(https://github.com/nodejs/lts), you can check [node.green](http://node.green)
for all available features in each release.
(https://github.com/nodejs/lts), [node.green](http://node.green) can be used
Copy link
Member

Choose a reason for hiding this comment

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

While we're in here, fix the comma splice? (The comma here should be a period and node.green is the start of a new sentence.)

For that matter, maybe eliminate the passive voice in that new sentence and add a normative trailing / to the URL?

node.green lists available features in each release.

@@ -274,8 +274,8 @@ These imported tests will be wrapped like this:
/* eslint-enable */
```

If you want to improve tests that have been imported this way, please send
a PR to the upstream project first. When your proposed change is merged in
In order to improve tests that have been imported this way, please send
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Remove In order:

To improve tests that have been imported this way, please send...

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.

Some nits, but LGTM

@mhdawson mhdawson changed the title doc: remove you from writing-tests.md doc: remove 'you' from writing-tests.md May 31, 2017
Noticed this while reading through writing-tests.md today.
As per style guide avoid the use of you, your etc.
Rational as per: http://www2.ivcc.edu/rambo/tip_formal_writing_voice.htm
@mhdawson
Copy link
Member Author

Pushed commits to address comments.

@mhdawson
Copy link
Member Author

mhdawson commented Jun 1, 2017

@mhdawson
Copy link
Member Author

mhdawson commented Jun 1, 2017

CI good, landed as 449dd71

mhdawson added a commit that referenced this pull request Jun 1, 2017
Noticed this while reading through writing-tests.md today.
As per style guide avoid the use of you, your etc.
Rational as per: http://www2.ivcc.edu/rambo/tip_formal_writing_voice.htm

PR-URL: #13319
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@mhdawson mhdawson closed this Jun 1, 2017
jasnell pushed a commit that referenced this pull request Jun 5, 2017
Noticed this while reading through writing-tests.md today.
As per style guide avoid the use of you, your etc.
Rational as per: http://www2.ivcc.edu/rambo/tip_formal_writing_voice.htm

PR-URL: #13319
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@jasnell jasnell mentioned this pull request Jun 5, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
@mhdawson mhdawson deleted the you-2 branch June 28, 2017 19:23
MylesBorins pushed a commit that referenced this pull request Jul 17, 2017
Noticed this while reading through writing-tests.md today.
As per style guide avoid the use of you, your etc.
Rational as per: http://www2.ivcc.edu/rambo/tip_formal_writing_voice.htm

PR-URL: #13319
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
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. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants