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: use precise terminology in test runner #50028

Merged
merged 2 commits into from
Oct 8, 2023
Merged

doc: use precise terminology in test runner #50028

merged 2 commits into from
Oct 8, 2023

Conversation

benjamingr
Copy link
Member

We currently use resolve which is incorrect from a technical point of view in several places in the test runner docs.

Most of the docs contain correct terminology.

For anyone wondering "resolves" means the promise's fate is known by either settling (becoming fulfilled or rejected) or because it's assimilating the status of another promise (that may be unfulfilled).

We currently use `resolve` which is incorrect from a technical point of view in several places in the test runner docs.

Most of the docs contain correct terminology.

For anyone wondering "resolves" means the promise's fate is known by either settling (becoming fulfilled or rejected) or because it's assimilating the status of another promise (that may be unfulfilled).
@benjamingr benjamingr added doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. labels Oct 3, 2023
@benjamingr benjamingr requested a review from MoLow October 3, 2023 17:35
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added the test_runner Issues and PRs related to the test runner subsystem. label Oct 3, 2023
benjamingr added a commit that referenced this pull request Oct 3, 2023
See #50028 for context - this also has the added benefit of reducing the amount of times we use resolving a promise vs resolving a path.

Note this document already used `fulfills` in many cases and I kept resolves (for promises) in some cases where it made sense to me from a technical point of view
Copy link
Member

@MoLow MoLow left a comment

Choose a reason for hiding this comment

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

LGTM, commit message should be docs and not doc

@benjamingr benjamingr removed the test Issues and PRs related to the tests. label Oct 3, 2023
@benjamingr
Copy link
Member Author

@MoLow thanks, I pushed from GitHub directly since I'm OOO and without my box I'll fix it when I'm back.

@aduh95 aduh95 merged commit d3a5f1f into main Oct 8, 2023
13 of 14 checks passed
@aduh95 aduh95 deleted the promses-doc-update branch October 8, 2023 10:05
@aduh95
Copy link
Contributor

aduh95 commented Oct 8, 2023

Landed in d3a5f1f

aduh95 pushed a commit that referenced this pull request Oct 8, 2023
See #50028 for context - this also
has the added benefit of reducing the amount of times we use resolving a
promise vs resolving a path.

Note this document already used `fulfills` in many cases and I kept
resolves (for promises) in some cases where it made sense to me from a
technical point of view.

PR-URL: #50029
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
We currently use `resolve` which is incorrect from a technical point of
view in several places in the test runner docs.

For anyone wondering "resolves" means the promise's fate is known by
either settling (becoming fulfilled or rejected) or because it's
assimilating the status of another promise (that may be unfulfilled).

PR-URL: nodejs#50028
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
See nodejs#50028 for context - this also
has the added benefit of reducing the amount of times we use resolving a
promise vs resolving a path.

Note this document already used `fulfills` in many cases and I kept
resolves (for promises) in some cases where it made sense to me from a
technical point of view.

PR-URL: nodejs#50029
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
targos pushed a commit that referenced this pull request Nov 11, 2023
We currently use `resolve` which is incorrect from a technical point of
view in several places in the test runner docs.

For anyone wondering "resolves" means the promise's fate is known by
either settling (becoming fulfilled or rejected) or because it's
assimilating the status of another promise (that may be unfulfilled).

PR-URL: #50028
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
targos pushed a commit that referenced this pull request Nov 11, 2023
See #50028 for context - this also
has the added benefit of reducing the amount of times we use resolving a
promise vs resolving a path.

Note this document already used `fulfills` in many cases and I kept
resolves (for promises) in some cases where it made sense to me from a
technical point of view.

PR-URL: #50029
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
We currently use `resolve` which is incorrect from a technical point of
view in several places in the test runner docs.

For anyone wondering "resolves" means the promise's fate is known by
either settling (becoming fulfilled or rejected) or because it's
assimilating the status of another promise (that may be unfulfilled).

PR-URL: nodejs#50028
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
See nodejs#50028 for context - this also
has the added benefit of reducing the amount of times we use resolving a
promise vs resolving a path.

Note this document already used `fulfills` in many cases and I kept
resolves (for promises) in some cases where it made sense to me from a
technical point of view.

PR-URL: nodejs#50029
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
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_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants