Skip to content

Commit

Permalink
doc: enhance guides by fixing and making grammar more consistent
Browse files Browse the repository at this point in the history
PR-URL: #33152
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
ChrisAHolland authored and codebytere committed Jun 9, 2020
1 parent 9f594be commit d2897a2
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 55 deletions.
58 changes: 30 additions & 28 deletions doc/guides/collaborator-guide.md
Expand Up @@ -109,8 +109,8 @@ review by @-mention.
See [Who to CC in the issue tracker](#who-to-cc-in-the-issue-tracker).

If you are the first Collaborator to approve a pull request that has no CI yet,
please [start one](#testing-and-ci). Please also start a new CI if the PR
creator pushed new code since the last CI run.
please [start one](#testing-and-ci). Please also start a new CI if the
pull request creator pushed new code since the last CI run.

### Consensus Seeking

Expand Down Expand Up @@ -269,22 +269,22 @@ master branch.

Examples of breaking changes include:

* removal or redefinition of existing API arguments
* changing return values
* removing or modifying existing properties on an options argument
* adding or removing errors
* altering expected timing of an event
* changing the side effects of using a particular API
* Removal or redefinition of existing API arguments.
* Changing return values.
* Removing or modifying existing properties on an options argument.
* Adding or removing errors.
* Altering expected timing of an event.
* Changing the side effects of using a particular API.

#### Breaking Changes and Deprecations

Existing stable public APIs that change in a backward-incompatible way must
undergo deprecation. The exceptions to this rule are:

* Adding or removing errors thrown or reported by a public API;
* Changing error messages for errors without error code;
* Altering the timing and non-internal side effects of the public API;
* Changes to errors thrown by dependencies of Node.js, such as V8;
* Adding or removing errors thrown or reported by a public API.
* Changing error messages for errors without error code.
* Altering the timing and non-internal side effects of the public API.
* Changes to errors thrown by dependencies of Node.js, such as V8.
* One-time exceptions granted by the TSC.

For more information, see [Deprecations](#deprecations).
Expand All @@ -310,7 +310,7 @@ after-the-fact.
Revert commits with `git revert <HASH>` or `git revert <FROM>..<TO>`. The
generated commit message will not have a subsystem and may violate line length
rules. That is OK. Append the reason for the revert and any `Refs` or `Fixes`
metadata. Raise a Pull Request like any other change.
metadata. Raise a pull request like any other change.

### Introducing New Modules

Expand Down Expand Up @@ -400,10 +400,10 @@ deprecation level of an API.
Collaborators may opt to elevate pull requests or issues to the [TSC][].
Do this if a pull request or issue:

* is labeled `semver-major`, or
* has a significant impact on the codebase, or
* is controversial, or
* is at an impasse among Collaborators who are participating in the discussion.
* Is labeled `semver-major`, or
* Has a significant impact on the codebase, or
* Is controversial, or
* Is at an impasse among Collaborators who are participating in the discussion.

@-mention the `@nodejs/tsc` GitHub team if you want to elevate an issue to the
[TSC][]. Do not use the GitHub UI on the right-hand side to assign to
Expand Down Expand Up @@ -573,7 +573,7 @@ for that commit. This is an opportunity to fix commit messages.
* The commit message text must conform to the [commit message guidelines][].
* <a name="metadata"></a>Change the original commit message to include metadata. (The
[`git node metadata`][git-node-metadata] command can generate the metadata
for you.)
for you).

* Required: A `PR-URL:` line that references the full GitHub URL of the pull
request. This makes it easy to trace a commit back to the conversation that
Expand All @@ -584,7 +584,8 @@ for that commit. This is an opportunity to fix commit messages.
background.
* Required: A `Reviewed-By: Name <email>` line for each Collaborator who
reviewed the change.
* Useful for @mentions / contact list if something goes wrong in the PR.
* Useful for @mentions / contact list if something goes wrong in the
pull request.
* Protects against the assumption that GitHub will be around forever.

Other changes may have landed on master since the successful CI run. As a
Expand All @@ -599,12 +600,13 @@ $ git rev-list upstream/master...HEAD | xargs core-validate-commit

Optional: For your own commits, force push the amended commit to the pull
request branch. If your branch name is `bugfix`, then: `git push
--force-with-lease origin master:bugfix`. Don't close the PR. It will close
after you push it upstream. It will have the purple merged status rather than
the red closed status. If you close the PR before GitHub adjusts its status, it
will show up as a 0 commit PR with no changed files. The order of operations is
important. If you push upstream before you push to your branch, GitHub will
close the issue with the red closed status.
--force-with-lease origin master:bugfix`. Don't close the pull request.
It will close after you push it upstream. It will have the purple merged
status rather than the red closed status. If you close the pull request
before GitHub adjusts its status, it will show up as a 0 commit pull
request with no changed files. The order of operations is important.
If you push upstream before you push to your branch, GitHub will close
the issue with the red closed status.

Time to push it:

Expand Down Expand Up @@ -645,7 +647,7 @@ git push upstream master
### I Made a Mistake

* Ping a TSC member.
* `#node-dev` on freenode
* `#node-dev` on freenode.
* With `git`, there's a way to override remote trees by force pushing
(`git push -f`). This is generally forbidden as it creates conflicts in other
people's forks. It is permissible for simpler slip-ups such as typos in commit
Expand Down Expand Up @@ -701,8 +703,8 @@ land on the staging branches, the backporter removes the `lts-watch-` label.
Likewise, as commits land in an LTS release, the releaser removes the `land-on-`
label.

Attach the appropriate `lts-watch-` label to any PR that may impact an LTS
release.
Attach the appropriate `lts-watch-` label to any pull request that
may impact an LTS release.

## Who to CC in the issue tracker

Expand Down
28 changes: 14 additions & 14 deletions doc/guides/cpp-style-guide.md
Expand Up @@ -40,13 +40,13 @@ runtime features.

Coding guidelines are based on the following guides (highest priority first):

1. This document
2. The [Google C++ Style Guide][]
3. The ISO [C++ Core Guidelines][]
1. This document.
2. The [Google C++ Style Guide][].
3. The ISO [C++ Core Guidelines][].

In general code should follow the C++ Core Guidelines, unless overridden by the
In general, code should follow the C++ Core Guidelines, unless overridden by the
Google C++ Style Guide or this document. At the moment these guidelines are
checked manually by reviewers, with the goal to validate this with automatic
checked manually by reviewers with the goal to validate this with automatic
tools.

## Formatting
Expand Down Expand Up @@ -282,11 +282,11 @@ data[0] = 12345;

### Type casting

* Use `static_cast<T>` if casting is required, and it is valid
* Use `reinterpret_cast` only when it is necessary
* Avoid C-style casts (`(type)value`)
* Use `static_cast<T>` if casting is required, and it is valid.
* Use `reinterpret_cast` only when it is necessary.
* Avoid C-style casts (`(type)value`).
* `dynamic_cast` does not work because Node.js is built without
[Run Time Type Information][]
[Run Time Type Information][].

Further reading:

Expand All @@ -313,13 +313,13 @@ for (const auto& item : some_map) {

### Do not include `*.h` if `*-inl.h` has already been included

Do
Do:

```cpp
#include "util-inl.h" // already includes util.h
```

instead of
Instead of:

```cpp
#include "util.h"
Expand Down Expand Up @@ -383,9 +383,9 @@ exports.foo = function(str) {
#### Avoid throwing JavaScript errors in nested C++ methods

When you need to throw a JavaScript exception from C++ (i.e.
`isolate()->ThrowException()`) prefer to do it as close to the return to JS as
possible, and not inside of nested C++ calls. Since this changes the JS
execution state doing it closest to where it is consumed reduces the chances of
`isolate()->ThrowException()`), do it as close to the return to JavaScript as
possible, and not inside of nested C++ calls. Since this changes the JavaScript
execution state, doing it closest to where it is consumed reduces the chances of
side effects.

Node.js is built [without C++ exception handling][], so code using `throw` or
Expand Down
4 changes: 2 additions & 2 deletions doc/guides/cve-management-process.md
Expand Up @@ -63,7 +63,7 @@ than two remaining CVEs a new block must be requested as follows:
to the Available list.

All changes to the files for managing CVEs in a given year will
be done through Pull Requests so that we have a record of how
be done through pull requests so that we have a record of how
the CVEs have been assigned.

CVEs are only valid for a specific year. At the beginning of each
Expand Down Expand Up @@ -122,7 +122,7 @@ following steps are used to assign, announce and report a CVE.
verification of the identity of the CVE submitter.

For each CVE listed, the additional data must include the following fields
updated with appropriate data for the CVE
updated with appropriate data for the CVE:
```text
[CVEID]: CVE-XXXX-XXXX
[PRODUCT]: Node.js
Expand Down
24 changes: 13 additions & 11 deletions doc/guides/maintaining-openssl.md
Expand Up @@ -3,10 +3,10 @@
This document describes how to update `deps/openssl/`.

## Requirements
* Linux environment
* Linux environment.
* `perl` Only Perl version 5 is tested.
* `nasm` (<http://www.nasm.us/>) The version of 2.11 or higher is needed.
* GNU `as` in binutils. The version of 2.26 or higher is needed.
* `nasm` (<http://www.nasm.us/>) Version 2.11 or higher is needed.
* GNU `as` in binutils. Version 2.26 or higher is needed.

## 0. Check Requirements

Expand Down Expand Up @@ -39,7 +39,8 @@ them.
% git commit openssl
````

The commit message can be (with the openssl version set to the relevant value):
The commit message can be written as (with the openssl version set
to the relevant value):
```text
deps: upgrade openssl sources to 1.1.0h
Expand All @@ -62,15 +63,15 @@ Use `make` to regenerate all platform dependent files in

## 3. Check diffs

Check diffs if updates are right. Even if no updates in openssl
sources, `buildinf.h` files will be updated for they have a timestamp
Check diffs to ensure updates are right. Even if there are no updates in openssl
sources, `buildinf.h` files will be updated because they have timestamp
data in them.
```sh
% git diff -- deps/openssl
```

*Note*: On Windows, OpenSSL Configure generates `makefile` that can be
used for `nmake` command. The `make` command in the step 2 above uses
*Note*: On Windows, OpenSSL Configure generates a `makefile` that can be
used for the `nmake` command. The `make` command in step 2 (above) uses
`Makefile_VC-WIN64A` and `Makefile_VC-WIN32` that are manually
created. When source files or build options are updated in Windows,
it needs to change these two Makefiles by hand. If you are not sure,
Expand All @@ -79,7 +80,7 @@ please ask @shigeki for details.
## 4. Commit and make test

Update all architecture dependent files. Do not forget to git add or remove
files if they are changed before commit:
files if they are changed before committing:
```sh
% git add deps/openssl/config/archs
% git add deps/openssl/openssl/include/crypto/bn_conf.h
Expand All @@ -88,7 +89,8 @@ files if they are changed before commit:
% git commit
```

The commit message can be (with the openssl version set to the relevant value):
The commit message can be written as (with the openssl version set
to the relevant value):
```text
deps: update archs files for OpenSSL-1.1.0
Expand All @@ -102,4 +104,4 @@ The commit message can be (with the openssl version set to the relevant value):
$ git commit
```

Finally, build Node.js and run tests.
Finally, build Node.js and run the tests.

0 comments on commit d2897a2

Please sign in to comment.