Skip to content

util: Use primordials.ArrayPrototypeIndexOf instead of mutable method#48586

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.

Sign up for GitHub

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

Jump to bottom
Merged
aduh95 merged 1 commit into from
Jul 15, 2023
Merged

util: Use primordials.ArrayPrototypeIndexOf instead of mutable method #48586

aduh95 merged 1 commit into from
Jul 15, 2023

Conversation

@DaisyDogs07
Copy link
Contributor

@DaisyDogs07DaisyDogs07 commented Jun 28, 2023•
edited
Loading

Found a part of identicalSequenceRange that uses a mutable method
Changed it to use the primordial instead

@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 Jun 28, 2023
aduh95
aduh95 approved these changes Jun 28, 2023
View reviewed changes
Copy link
Contributor

@aduh95aduh95 left a comment

Choose a reason for hiding this comment

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

Thanks! Can you amend the commit message to comply with our guidelines? Something like util: use `primordials.ArrayPrototypeIndexOf` instead of mutable method

@DaisyDogs07DaisyDogs07 changed the title Use primordial ArrayPrototypeIndexOf in identicalSequenceRangeutil: Use primordial ArrayPrototypeIndexOf in identicalSequenceRangeJun 28, 2023
@DaisyDogs07DaisyDogs07 changed the title util: Use primordial ArrayPrototypeIndexOf in identicalSequenceRangeutil: Use primordials.ArrayPrototypeIndexOf in identicalSequenceRangeJun 28, 2023
@DaisyDogs07
Copy link
ContributorAuthor

DaisyDogs07 commented Jun 28, 2023

Sorry about that. First time making a pull request or anything for something big :/

@DaisyDogs07DaisyDogs07 changed the title util: Use primordials.ArrayPrototypeIndexOf in identicalSequenceRangeutil: Use primordials.ArrayPrototypeIndexOf instead of mutable methodJun 28, 2023
@aduh95
Copy link
Contributor

aduh95 commented Jun 29, 2023

I can see you changed the PR title, but unfortunately that's not what's needed: you need to amend the commit message, and then force push to your PR branch. Please make sure to also follow this rule:

node/doc/contributing/pull-requests.md

Lines 166 to 167 in 260092e

* be entirely in lowercase with the exception of proper nouns, acronyms, and
the words that refer to code, like function/variable names

Let me know if you need help with that, I can do it for you if that's more convenient.

@DaisyDogs07
Copy link
ContributorAuthor

DaisyDogs07 commented Jun 29, 2023•
edited
Loading

Unfortunately im new to PRs and i dont know how to force push something. may i ask if you could do it for me?

@DaisyDogs07@aduh95
util: use primordials.ArrayPrototypeIndexOf instead of mutable method
29b519e
I believe you forgot to use the primordial `ArrayPrototypeIndexOf` in `identicalSequenceRange` i dont know if this was intentional but im fixing it anyway :p
@aduh95aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 30, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 30, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/52565/

@aduh95aduh95 merged commit 4a26e2d into nodejs:mainJul 15, 2023
@aduh95
Copy link
Contributor

aduh95 commented Jul 15, 2023

Landed in 4a26e2d, thanks for the contribution 🎉

Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@DaisyDogs07@Ceres6
util: use primordials.ArrayPrototypeIndexOf instead of mutable method
094b106
PR-URL: nodejs#48586 Reviewed-By: Antoine du Hamel <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@DaisyDogs07@Ceres6
util: use primordials.ArrayPrototypeIndexOf instead of mutable method
cbf32f5
PR-URL: nodejs#48586 Reviewed-By: Antoine du Hamel <[email protected]>
UlisesGascon pushed a commit to UlisesGascon/node that referenced this pull request Aug 14, 2023
@DaisyDogs07@UlisesGascon
util: use primordials.ArrayPrototypeIndexOf instead of mutable method
0f5d6d7
PR-URL: nodejs#48586 Reviewed-By: Antoine du Hamel <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Aug 15, 2023
@DaisyDogs07@RafaelGSS
util: use primordials.ArrayPrototypeIndexOf instead of mutable method
c3dbd0c
PR-URL: #48586 Reviewed-By: Antoine du Hamel <[email protected]>
@UlisesGasconUlisesGascon mentioned this pull request Aug 15, 2023
Merged
ruyadorno pushed a commit that referenced this pull request Sep 12, 2023
@DaisyDogs07@ruyadorno
util: use primordials.ArrayPrototypeIndexOf instead of mutable method
fabaff6
PR-URL: #48586 Reviewed-By: Antoine du Hamel <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 13, 2023
@DaisyDogs07@ruyadorno
util: use primordials.ArrayPrototypeIndexOf instead of mutable method
66fb032
PR-URL: #48586 Reviewed-By: Antoine du Hamel <[email protected]>
@ruyadornoruyadorno mentioned this pull request Sep 13, 2023
Merged
ruyadorno pushed a commit that referenced this pull request Sep 17, 2023
@DaisyDogs07@ruyadorno
util: use primordials.ArrayPrototypeIndexOf instead of mutable method
0a022c4
PR-URL: #48586 Reviewed-By: Antoine du Hamel <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95aduh95aduh95 approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.needs-ciPRs that need a full CI run.utilIssues and PRs related to the built-in util module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DaisyDogs07@aduh95@nodejs-github-bot