-
Notifications
You must be signed in to change notification settings - Fork 30k
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
benchmark: use cluster.isPrimary
instead of cluster.isMaster
#48002
benchmark: use cluster.isPrimary
instead of cluster.isMaster
#48002
Conversation
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.
In benchmarks, this makes sense. In tests, there are already various occurrences of isPrimary
, and removing all usages of the old APIs means that they won't be covered by tests anymore. We should still be testing deprecated APIs until they are removed.
`cluster.isMaster` was deprecated. So need to use `cluster.isPrimary` for benchmark. Refs: nodejs#47981
7fa0309
to
fab804c
Compare
cluster.isPrimary
instead of cluster.isMaster
cluster.isPrimary
instead of cluster.isMaster
I agree with you. So revert changed code for test. Thank you for kind review! |
This comment was marked as outdated.
This comment was marked as outdated.
Commit Queue failed- Loading data for nodejs/node/pull/48002 ✔ Done loading data for nodejs/node/pull/48002 ----------------------------------- PR info ------------------------------------ Title benchmark: use `cluster.isPrimary` instead of `cluster.isMaster` (#48002) Author Deokjin Kim (@deokjinkim) Branch deokjinkim:230514_benchmark_use_cluster_isprimary -> nodejs:main Labels cluster, benchmark, author ready, needs-ci Commits 1 - benchmark: use `cluster.isPrimary` instead of `cluster.isMaster` Committers 1 - Deokjin Kim PR-URL: https://github.com/nodejs/node/pull/48002 Refs: https://github.com/nodejs/node/pull/47981 Reviewed-By: Moshe Atlow Reviewed-By: Tobias Nießen Reviewed-By: Luigi Pinca ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/48002 Refs: https://github.com/nodejs/node/pull/47981 Reviewed-By: Moshe Atlow Reviewed-By: Tobias Nießen Reviewed-By: Luigi Pinca -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 14 May 2023 13:59:40 GMT ✔ Approvals: 3 ✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/48002#pullrequestreview-1425560993 ✔ - Tobias Nießen (@tniessen) (TSC): https://github.com/nodejs/node/pull/48002#pullrequestreview-1425565383 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/48002#pullrequestreview-1427261292 ✘ Last GitHub CI failed ℹ Last Full PR CI on 2023-05-15T13:01:01Z: https://ci.nodejs.org/job/node-test-pull-request/51817/ - Querying data for job/node-test-pull-request/51817/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/4992884399 |
Commit Queue failed- Loading data for nodejs/node/pull/48002 ✔ Done loading data for nodejs/node/pull/48002 ----------------------------------- PR info ------------------------------------ Title benchmark: use `cluster.isPrimary` instead of `cluster.isMaster` (#48002) Author Deokjin Kim (@deokjinkim) Branch deokjinkim:230514_benchmark_use_cluster_isprimary -> nodejs:main Labels cluster, benchmark, author ready, needs-ci Commits 1 - benchmark: use `cluster.isPrimary` instead of `cluster.isMaster` Committers 1 - Deokjin Kim PR-URL: https://github.com/nodejs/node/pull/48002 Refs: https://github.com/nodejs/node/pull/47981 Reviewed-By: Moshe Atlow Reviewed-By: Tobias Nießen Reviewed-By: Luigi Pinca ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/48002 Refs: https://github.com/nodejs/node/pull/47981 Reviewed-By: Moshe Atlow Reviewed-By: Tobias Nießen Reviewed-By: Luigi Pinca -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 14 May 2023 13:59:40 GMT ✔ Approvals: 3 ✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/48002#pullrequestreview-1425560993 ✔ - Tobias Nießen (@tniessen) (TSC): https://github.com/nodejs/node/pull/48002#pullrequestreview-1425565383 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/48002#pullrequestreview-1427261292 ✘ Last GitHub CI failed ℹ Last Full PR CI on 2023-05-16T14:04:27Z: https://ci.nodejs.org/job/node-test-pull-request/51817/ - Querying data for job/node-test-pull-request/51817/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/4998309251 |
Landed in ca09656 |
`cluster.isMaster` was deprecated. So need to use `cluster.isPrimary` for benchmark. Refs: #47981 PR-URL: #48002 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
`cluster.isMaster` was deprecated. So need to use `cluster.isPrimary` for benchmark. Refs: #47981 PR-URL: #48002 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
`cluster.isMaster` was deprecated. So need to use `cluster.isPrimary` for benchmark. Refs: nodejs#47981 PR-URL: nodejs#48002 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
cluster.isMaster
was deprecated. So need to usecluster.isPrimary
for benchmark.Refs: #47981