From d2897a28367cf77e33108bc068e91ff9ba0ac7fd Mon Sep 17 00:00:00 2001 From: Chris Holland Date: Wed, 29 Apr 2020 16:38:16 -0700 Subject: [PATCH] doc: enhance guides by fixing and making grammar more consistent PR-URL: https://github.com/nodejs/node/pull/33152 Reviewed-By: Anna Henningsen Reviewed-By: Michael Dawson --- doc/guides/collaborator-guide.md | 58 ++++++++++++++-------------- doc/guides/cpp-style-guide.md | 28 +++++++------- doc/guides/cve-management-process.md | 4 +- doc/guides/maintaining-openssl.md | 24 ++++++------ 4 files changed, 59 insertions(+), 55 deletions(-) diff --git a/doc/guides/collaborator-guide.md b/doc/guides/collaborator-guide.md index 128f06857b8f0a..11ca506dacee3e 100644 --- a/doc/guides/collaborator-guide.md +++ b/doc/guides/collaborator-guide.md @@ -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 @@ -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). @@ -310,7 +310,7 @@ after-the-fact. Revert commits with `git revert ` or `git revert ..`. 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 @@ -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 @@ -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][]. * 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 @@ -584,7 +584,8 @@ for that commit. This is an opportunity to fix commit messages. background. * Required: A `Reviewed-By: Name ` 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 @@ -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: @@ -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 @@ -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 diff --git a/doc/guides/cpp-style-guide.md b/doc/guides/cpp-style-guide.md index f937b60edc20d4..32a178ea81256c 100644 --- a/doc/guides/cpp-style-guide.md +++ b/doc/guides/cpp-style-guide.md @@ -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 @@ -282,11 +282,11 @@ data[0] = 12345; ### Type casting -* Use `static_cast` 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` 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: @@ -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" @@ -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 diff --git a/doc/guides/cve-management-process.md b/doc/guides/cve-management-process.md index eba94484b53877..fbd7f2cb84a3be 100644 --- a/doc/guides/cve-management-process.md +++ b/doc/guides/cve-management-process.md @@ -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 @@ -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 diff --git a/doc/guides/maintaining-openssl.md b/doc/guides/maintaining-openssl.md index af59486b0f3219..6a584f46024171 100644 --- a/doc/guides/maintaining-openssl.md +++ b/doc/guides/maintaining-openssl.md @@ -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` () The version of 2.11 or higher is needed. -* GNU `as` in binutils. The version of 2.26 or higher is needed. +* `nasm` () Version 2.11 or higher is needed. +* GNU `as` in binutils. Version 2.26 or higher is needed. ## 0. Check Requirements @@ -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 @@ -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, @@ -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 @@ -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 @@ -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.