-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
test: fix typos #55063
test: fix typos #55063
Conversation
Review requested:
|
@NathanBaulch thank you for your first contribution! I only skimmed through the changed files to see if anything stands out, don't take this as a review of your changes as a whole, just this bit: Please revert all changes made in the |
I would also recommend to split the remaining typo fixes into 3 separate PRs
|
bd182e6
to
45ca0fd
Compare
Sure, we can start with the comment typos in this PR @panva - I've restored all the non-comment ones. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #55063 +/- ##
=======================================
Coverage 88.24% 88.25%
=======================================
Files 651 651
Lines 183877 183877
Branches 35853 35858 +5
=======================================
+ Hits 162269 162281 +12
+ Misses 14899 14892 -7
+ Partials 6709 6704 -5 |
test/common/index.js
Outdated
@@ -171,7 +171,7 @@ const buildType = process.config.target_defaults ? | |||
|
|||
// If env var is set then enable async_hook hooks for all tests. | |||
if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) { | |||
const destroydIdsList = {}; | |||
const destroyIdsList = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be "destroy" or "destroyed"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure, happy to change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Commit Queue failed- Loading data for nodejs/node/pull/55063 ✔ Done loading data for nodejs/node/pull/55063 ----------------------------------- PR info ------------------------------------ Title test: fix typos (#55063) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch NathanBaulch:typos-test -> nodejs:main Labels test, addons, esm, author ready, needs-ci Commits 2 - test: fix typos - test: restore non-comment typos Committers 1 - Nathan Baulch <[email protected]> PR-URL: https://github.com/nodejs/node/pull/55063 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Matteo Collina <[email protected]> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/55063 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Matteo Collina <[email protected]> -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 22 Sep 2024 11:01:35 GMT ✔ Approvals: 2 ✔ - Filip Skokan (@panva): https://github.com/nodejs/node/pull/55063#pullrequestreview-2324365793 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/55063#pullrequestreview-2334977790 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-09-27T15:26:50Z: https://ci.nodejs.org/job/node-test-pull-request/62803/ - Querying data for job/node-test-pull-request/62803/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD 18acff0d01..f5d454ac7e main -> origin/main ✔ origin/main is now up-to-date main is out of sync with origin/main. Mismatched commits: - 5aadbb8ad2 src: add receiver to fast api callback methods - f5d454ac7e src: add receiver to fast api callback methods -------------------------------------------------------------------------------- HEAD is now at f5d454ac7e src: add receiver to fast api callback methods ✔ Reset to origin/main - Downloading patch for 55063 From https://github.com/nodejs/node * branch refs/pull/55063/merge -> FETCH_HEAD ✔ Fetched commits as f5d454ac7e6b..45ca0fd106d8 -------------------------------------------------------------------------------- Auto-merging test/es-module/test-typescript.mjs [main 1348c25f7f] test: fix typos Author: Nathan Baulch <[email protected]> Date: Sun Sep 22 15:03:43 2024 +1000 72 files changed, 275 insertions(+), 275 deletions(-) rename test/benchmark/{test-bechmark-readline.js => test-benchmark-readline.js} (100%) Auto-merging test/es-module/test-typescript.mjs [main f1e42cb278] test: restore non-comment typos Author: Nathan Baulch <[email protected]> Date: Tue Sep 24 11:56:41 2024 +1000 33 files changed, 231 insertions(+), 231 deletions(-) rename test/benchmark/{test-benchmark-readline.js => test-bechmark-readline.js} (100%) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. Rebasing (2/4)https://github.com/nodejs/node/actions/runs/11082939946 |
Landed in 28c7394 |
PR-URL: #55063 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
PR-URL: #55063 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs#55063 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Just thought I'd contribute some typo fixes that I stumbled on. Nothing controversial (hopefully).
Use the following command to get a quick summary of the specific corrections made:
FWIW, the top typos are: