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

util: use hasOwnProperty() primordial #41692

Merged
merged 1 commit into from
Jan 27, 2022
Merged

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
@nodejs-github-bot
Copy link
Collaborator

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
@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
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants