GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — util: Use `primordials.ArrayPrototypeIndexOf` instead of mutable method by DaisyDogs07 · Pull Request #48586 · nodejs/node

Conversation

@DaisyDogs07

@DaisyDogs07 DaisyDogs07 commented Jun 28, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@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 Jun 28, 2023
aduh95
aduh95 approved these changes Jun 28, 2023
View reviewed changes

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@DaisyDogs07 DaisyDogs07 changed the title Use primordial ArrayPrototypeIndexOf in identicalSequenceRange util: Use primordial ArrayPrototypeIndexOf in identicalSequenceRange Jun 28, 2023
@DaisyDogs07 DaisyDogs07 changed the title util: Use primordial ArrayPrototypeIndexOf in identicalSequenceRange util: Use primordials.ArrayPrototypeIndexOf in identicalSequenceRange Jun 28, 2023
@DaisyDogs07

DaisyDogs07 commented Jun 28, 2023

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@DaisyDogs07 DaisyDogs07 changed the title util: Use primordials.ArrayPrototypeIndexOf in identicalSequenceRange util: Use primordials.ArrayPrototypeIndexOf instead of mutable method Jun 28, 2023
@aduh95

aduh95 commented Jun 29, 2023

Copy link
Copy Markdown
Contributor

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@DaisyDogs07

DaisyDogs07 commented Jun 29, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Jun 30, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jun 30, 2023
@nodejs-github-bot

nodejs-github-bot commented Jun 30, 2023

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95
aduh95 merged commit 4a26e2d into nodejs:main Jul 15, 2023
@aduh95

aduh95 commented Jul 15, 2023

Copy link
Copy Markdown
Contributor

Landed in 4a26e2d, thanks for the contribution 🎉
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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]>
@UlisesGascon UlisesGascon mentioned this pull request Aug 15, 2023
v20.6.0 proposal #49185
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]>
@ruyadorno ruyadorno mentioned this pull request Sep 13, 2023
v18.18.0 proposal #49220
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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95 aduh95 aduh95 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. needs-ci PRs that need a full CI run. util Issues 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.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@DaisyDogs07 @aduh95 @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.