GeezerVison

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

GeezerVison is showing github.com — lib: add RegExp primordials by Sebastien-Ahkrin · Pull Request #31208 · nodejs/node

Conversation

Sebastien-Ahkrin commented Jan 6, 2020

Copy link

Copy Markdown

Contributor

Hello,
For this PR I have added RegExp in the primordials eslint
And i just have created a line in "/lib/.eslintrc.yaml".

rules:
  no-restricted-globals:
  - name: RegExp 
      message: "Use `const { RegExp } = primordials;` instead of the global."

And just add RegExp.

const {
  // [...]
  RegExp,
} = primordials;

I hope this new PR will help you :x

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


          lib: add RegExp primordials

16c3c63

nodejs-github-bot added the lib / src Issues and PRs involving general changes in the lib/ or src/ directories. label

Jan 6, 2020

Sebastien-Ahkrin commented Jan 6, 2020

Copy link

Copy Markdown

Contributor Author

Refs: #30697

All reactions

Sorry, something went wrong.

Uh oh!

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

targos approved these changes

Jan 6, 2020

View reviewed changes

cjihrig approved these changes

Jan 6, 2020

View reviewed changes

trivikr reviewed

Jan 6, 2020

View reviewed changes

trivikr left a comment

Copy link

Copy Markdown

Member

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.

Is file print_help.js missed or not required?

node/lib/internal/main/print_help.js

Line 67 in 20fd123

return text.replace(new RegExp(`([^\n]{0,${width}})( |$)`, 'g'),

Sorry, something went wrong.

Uh oh!

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

All reactions

jasnell approved these changes

Jan 6, 2020

View reviewed changes

BridgeAR approved these changes

Jan 6, 2020

View reviewed changes

nodejs-github-bot commented Jan 6, 2020

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

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

Trott approved these changes

Jan 7, 2020

View reviewed changes

Trott commented Jan 7, 2020

Copy link

Copy Markdown

Member

Is file print_help.js missed or not required?

node/lib/internal/main/print_help.js

Line 67 in 20fd123

return text.replace(new RegExp(`([^\n]{0,${width}})( |$)`, 'g'),

It has a comment disabling the no-restricted-globals rule so maybe using git blame to look into why that was added would answer the question.

All reactions

Sorry, something went wrong.

Uh oh!

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

targos commented Jan 7, 2020

Copy link

Copy Markdown

Member

Apparently it was me who added this comment. I guess at that time that the file didn't have access to primordials. I don't know about now.

Sebastien-Ahkrin and EduenSarceno reacted with laugh emoji

All reactions

😄 2 reactions

Sorry, something went wrong.

Uh oh!

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

Trott commented Jan 8, 2020

Copy link

Copy Markdown

Member

Apparently it was me who added this comment. I guess at that time that the file didn't have access to primordials. I don't know about now.

That file can probably be done separately in its own PR removing the ESLint comment and updating as appropriate.

All reactions

Sorry, something went wrong.

Uh oh!

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

nodejs-github-bot commented Jan 8, 2020

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

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

trivikr approved these changes

Jan 8, 2020

View reviewed changes

Trott commented Jan 8, 2020

Copy link

Copy Markdown

Member

Landed in a494d12

All reactions

Sorry, something went wrong.

Uh oh!

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

Trott closed this

Jan 8, 2020

Trott pushed a commit that referenced this pull request

Jan 8, 2020


          lib: add RegExp primordials

a494d12

PR-URL: #31208
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>

MylesBorins pushed a commit that referenced this pull request

Jan 16, 2020


          lib: add RegExp primordials

53e73fc

PR-URL: #31208
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>

codebytere mentioned this pull request

Jan 16, 2020

v13.7.0 proposal #31382

Merged

codebytere pushed a commit that referenced this pull request

Mar 14, 2020


          lib: add RegExp primordials

bc4cbe3

PR-URL: #31208
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>

codebytere mentioned this pull request

Mar 17, 2020

v12.16.2 proposal #32313

Merged

Reviewers

jasnell

jasnell approved these changes

Trott

Trott approved these changes

cjihrig

cjihrig approved these changes

BridgeAR

BridgeAR approved these changes

trivikr

trivikr approved these changes

+1 more reviewer

targos

targos approved these changes

Reviewers whose approvals may not affect merge requirements

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.

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.

8 participants

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.