Skip to content

Conversation

@Trott
Copy link
Member

@Trott Trott commented Jan 25, 2022

Use primordial ObjectPrototypeHasOwnProperty primordial.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Jan 25, 2022
@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2022
@Trott

This comment has been minimized.

@Trott Trott removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2022
Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Does this need don't backport labels?

@Trott Trott changed the title inspector: use ObjectHasOwn primordial util: use ObjectHasOwn primordial Jan 25, 2022
@Trott
Copy link
Member Author

Trott commented Jan 25, 2022

Does this need don't backport labels?

Added. Thanks.

@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2022
Copy link
Contributor

@bnb bnb left a comment

Choose a reason for hiding this comment

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

this looks right to me :)

@Trott Trott changed the title util: use ObjectHasOwn primordial util: use hasOwnProperty() primordial Jan 25, 2022
@benjamingr
Copy link
Member

May also want to edit the commit message ( use hasOwnProperty() primordial -> use ObjectHasOwn primordial ?)

@Trott
Copy link
Member Author

Trott commented Jan 26, 2022

May also want to edit the commit message ( use hasOwnProperty() primordial -> use ObjectHasOwn primordial ?)

Done, but the other way around. There is no ObjectHasOwn primordial which I guess makes sense because it would be the same as the ObjectPrototypeHasOwnProperty primordial--same arguments and everything. But I'm not sure why ObjectHasOwn doesn't exist. So I just switched it to ObjectPrototypeHasOwnProperty. ¯\(ツ)

@benjamingr
Copy link
Member

@Trott then this is probably fine to backport?

@Trott
Copy link
Member Author

Trott commented Jan 26, 2022

@Trott then this is probably fine to backport?

Oh, yes, good point! I'll remove those labels.

@Trott Trott added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 27, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 27, 2022
@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jan 27, 2022
@nodejs-github-bot

This comment has been minimized.

@Trott Trott added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jan 27, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 27, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/41692 ✔ Done loading data for nodejs/node/pull/41692 ----------------------------------- PR info ------------------------------------ Title util: use hasOwnProperty() primordial (#41692) Author Rich Trott (@Trott) Branch Trott:hasown-inspector -> nodejs:master Labels util, needs-ci Commits 1 - util: use hasOwnProperty() primordial Committers 1 - Rich Trott PR-URL: https://github.com/nodejs/node/pull/41692 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Tierney Cyren Reviewed-By: Luigi Pinca ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/41692 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Tierney Cyren Reviewed-By: Luigi Pinca -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last review: ⚠ - util: use hasOwnProperty() primordial ℹ This PR was created on Tue, 25 Jan 2022 15:16:03 GMT ✔ Approvals: 3 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/41692#pullrequestreview-862441837 ✔ - Tierney Cyren (@bnb): https://github.com/nodejs/node/pull/41692#pullrequestreview-862639477 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/41692#pullrequestreview-862732432 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2022-01-27T05:45:45Z: https://ci.nodejs.org/job/node-test-pull-request/42158/ - Querying data for job/node-test-pull-request/42158/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/1758126043
@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jan 27, 2022
Avoid Object.prototype.hasOwnProperty. Use primordial instead. PR-URL: nodejs#41692 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Trott
Copy link
Member Author

Trott commented Jan 27, 2022

Landed in acc92a7

@Trott Trott merged commit acc92a7 into nodejs:master Jan 27, 2022
@Trott Trott deleted the hasown-inspector branch January 27, 2022 19:50
Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
Avoid Object.prototype.hasOwnProperty. Use primordial instead. PR-URL: nodejs#41692 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
ruyadorno pushed a commit that referenced this pull request Feb 8, 2022
Avoid Object.prototype.hasOwnProperty. Use primordial instead. PR-URL: #41692 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Mar 2, 2022
Avoid Object.prototype.hasOwnProperty. Use primordial instead. PR-URL: #41692 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Mar 3, 2022
Avoid Object.prototype.hasOwnProperty. Use primordial instead. PR-URL: #41692 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Mar 14, 2022
Avoid Object.prototype.hasOwnProperty. Use primordial instead. PR-URL: #41692 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-failed An error occurred while landing this pull request using GitHub Actions. needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module.

5 participants