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 — [v18.x backport] lib: reset `RegExp` statics before running user code by aduh95 · Pull Request #44247 · nodejs/node

Conversation

@aduh95

@aduh95 aduh95 commented Aug 16, 2022

Copy link
Copy Markdown
Contributor

Fixes: #43740

PR-URL: #43741
Reviewed-By: Ruben Bridgewater [email protected]
Reviewed-By: Geoffrey Booth [email protected]
Reviewed-By: Matteo Collina [email protected]

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 commented Aug 16, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/modules
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. v18.x labels Aug 16, 2022
@aduh95

aduh95 commented Aug 16, 2022

Copy link
Copy Markdown
Contributor Author

It looks like the lint failure was introduced by 5cb5c65.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95 aduh95 added the blocked PRs that are blocked by other issues or PRs. label Aug 16, 2022
This was referenced Aug 16, 2022
lib: add diagnostics channel and perf hooks detail #43984
Merged
[v18.x] src: split property helpers from node::Environment #44251
Closed
@danielleadams
danielleadams force-pushed the v18.x-staging branch from 96c3373 to e028edb Compare August 17, 2022 01:16
@danielleadams

danielleadams commented Aug 17, 2022

Copy link
Copy Markdown
Contributor

@aduh95 I had to rest v18.x-staging, sorry but this will need a rebase
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95
lib: reset RegExp statics before running user code
aa76869 
Fixes: nodejs#43740

PR-URL: nodejs#43741
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@aduh95
aduh95 force-pushed the backport-regexp-statics-great-reset branch from 48d92db to aa76869 Compare August 23, 2022 16:58
@aduh95 aduh95 added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed blocked PRs that are blocked by other issues or PRs. labels Aug 23, 2022
@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 Aug 23, 2022
@nodejs-github-bot

nodejs-github-bot commented Aug 23, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
This was referenced Aug 24, 2022
CI Reliability 2022-08-24 nodejs/reliability#356
Open
CI Reliability 2022-08-25 nodejs/reliability#357
Open
CI Reliability 2022-08-26 nodejs/reliability#358
Open
CI Reliability 2022-08-27 nodejs/reliability#359
Open
CI Reliability 2022-08-28 nodejs/reliability#360
Open
@nodejs-github-bot

nodejs-github-bot commented Sep 5, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Sep 5, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Sep 6, 2022
CI Reliability 2022-09-06 nodejs/reliability#369
Open
17 tasks
@nodejs-github-bot

nodejs-github-bot commented Sep 6, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Sep 6, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@RafaelGSS
RafaelGSS force-pushed the v18.x-staging branch from d912d2d to 60191c6 Compare September 6, 2022 16:34
@nodejs-github-bot

nodejs-github-bot commented Sep 6, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Sep 7, 2022
CI Reliability 2022-09-07 nodejs/reliability#370
Open
15 tasks
@RafaelGSS
RafaelGSS force-pushed the v18.x-staging branch from 60191c6 to c0cfb14 Compare September 7, 2022 12:21
@nodejs-github-bot

nodejs-github-bot commented Sep 7, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Sep 7, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@RafaelGSS
RafaelGSS force-pushed the v18.x-staging branch from c0cfb14 to 8ef5c40 Compare September 7, 2022 16:28
@github-actions github-actions Bot mentioned this pull request Sep 8, 2022
CI Reliability 2022-09-08 nodejs/reliability#371
Open
12 tasks
@nodejs-github-bot

nodejs-github-bot commented Sep 8, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
This was referenced Sep 9, 2022
CI Reliability 2022-09-09 nodejs/reliability#372
Open
CI Reliability 2022-09-10 nodejs/reliability#373
Open
CI Reliability 2022-09-11 nodejs/reliability#374
Open
CI Reliability 2022-09-12 nodejs/reliability#375
Open
CI Reliability 2022-09-13 nodejs/reliability#376
Open
@nodejs-github-bot

nodejs-github-bot commented Sep 16, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 16, 2022
@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 Sep 16, 2022
@nodejs-github-bot

nodejs-github-bot commented Sep 16, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

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

RafaelGSS commented Sep 23, 2022

Copy link
Copy Markdown
Member

@aduh95 Is it ready to land? I'm about to including it to the v18 proposal
All reactions

Sorry, something went wrong.

Uh oh!

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

aduh95 commented Sep 23, 2022

Copy link
Copy Markdown
Contributor Author

It is ready to land, thanks for taking care of this :)
All reactions

Sorry, something went wrong.

Uh oh!

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

RafaelGSS commented Sep 23, 2022

Copy link
Copy Markdown
Member

It is ready to land, thanks for taking care of this :)

Can you amend the Backport-PR-URL metadata to your commit message?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
RafaelGSS
RafaelGSS approved these changes Sep 23, 2022
View reviewed changes
@aduh95

aduh95 commented Sep 23, 2022

Copy link
Copy Markdown
Contributor Author

It is ready to land, thanks for taking care of this :)

Can you amend the Backport-PR-URL metadata to your commit message?

Sure I can, but I find it surprising as of all the backport PRs I've opened, I've never touched the commit message, and it was taken care of upon landing – also if I push another commit, that means we'd probably need another CI run, which I'd rather avoid. Are you sure that's not something that ncu takes care of?
All reactions

Sorry, something went wrong.

Uh oh!

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

RafaelGSS commented Sep 23, 2022

Copy link
Copy Markdown
Member

@nodejs/releasers Do ncu handle it by default? I'm just asking because following the https://github.com/nodejs/node/blob/main/doc/contributing/backporting-to-release-lines.md#how-to-submit-a-backport-pull-request step 9.v says:

Amend the commit message and include a Backport-PR-URL: metadata and re-push the change to your fork.

I think I can include that metadata while merging using git node land X, but I'm not quite sure if that's the correct workflow.
All reactions

Sorry, something went wrong.

Uh oh!

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

RafaelGSS commented Sep 23, 2022

Copy link
Copy Markdown
Member

Landed in 687ffcc
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@RafaelGSS RafaelGSS closed this Sep 23, 2022
RafaelGSS pushed a commit that referenced this pull request Sep 23, 2022
@aduh95 @RafaelGSS
lib: reset RegExp statics before running user code
687ffcc 
Fixes: #43740

Backport-PR-URL: #43741
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
PR-URL: #44247
Reviewed-By: Rafael Gonzaga <[email protected]>
@RafaelGSS

RafaelGSS commented Sep 23, 2022

Copy link
Copy Markdown
Member

@aduh95 just realized that I can do it manually using ncu. Sorry for the inconvenience.
aduh95 reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

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

richardlau commented Sep 23, 2022

Copy link
Copy Markdown
Member

@nodejs/releasers Do ncu handle it by default? I'm just asking because following the https://github.com/nodejs/node/blob/main/doc/contributing/backporting-to-release-lines.md#how-to-submit-a-backport-pull-request step 9.v says:

Amend the commit message and include a Backport-PR-URL: metadata and re-push the change to your fork.

I think I can include that metadata while merging using git node land X, but I'm not quite sure if that's the correct workflow.

Use --backport with git node land (nodejs/node-core-utils#383).
RafaelGSS reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@RafaelGSS RafaelGSS mentioned this pull request Sep 23, 2022
doc: mention git node backport #44764
Closed
@aduh95
aduh95 deleted the backport-regexp-statics-great-reset branch September 24, 2022 12:30
@RafaelGSS RafaelGSS mentioned this pull request Sep 26, 2022
v18.10.0 proposal #44799
Merged
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
@aduh95 @RafaelGSS
lib: reset RegExp statics before running user code
d8d34ae 
Fixes: #43740

Backport-PR-URL: #43741
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
PR-URL: #44247
Reviewed-By: Rafael Gonzaga <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RafaelGSS RafaelGSS RafaelGSS approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

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.

5 participants

@aduh95 @nodejs-github-bot @danielleadams @RafaelGSS @richardlau
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.