Skip to content

Conversation

@Trott
Copy link
Member

@TrottTrott commented Jan 25, 2022

Use primordial ObjectPrototypeHasOwnProperty primordial.

@nodejs-github-botnodejs-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
@TrottTrott 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.

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

@benjamingrbenjamingr 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?

@TrottTrott changed the title inspector: use ObjectHasOwn primordialutil: use ObjectHasOwn primordialJan 25, 2022
@Trott
Copy link
MemberAuthor

Does this need don't backport labels?

Added. Thanks.

@TrottTrott added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2022
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2022
@nodejs-github-bot
Copy link
Collaborator

bnb
bnb approved these changes Jan 25, 2022
Copy link
Contributor

@bnbbnb 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 :)

@TrottTrott changed the title util: use ObjectHasOwn primordialutil: use hasOwnProperty() primordial Jan 25, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@benjamingr
Copy link
Member

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

@Trott
Copy link
MemberAuthor

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
MemberAuthor

@Trott then this is probably fine to backport?

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

@TrottTrott added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 27, 2022
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 27, 2022
@nodejs-github-botnodejs-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.

@TrottTrott 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-botnodejs-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-botnodejs-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 <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@Trott
Copy link
MemberAuthor

Landed in acc92a7

@TrottTrott merged commit acc92a7 into nodejs:masterJan 27, 2022
@TrottTrott 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 <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
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 <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@ruyadornoruyadorno mentioned this pull request Feb 8, 2022
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 <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
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 <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
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 <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Trott@nodejs-github-bot@benjamingr@bnb@lpinca