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.

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

benchmark, libs: remove needless RegExp capturing #13718

Closed
wants to merge 2 commits into from
Closed

benchmark, libs: remove needless RegExp capturing #13718

wants to merge 2 commits into from

Conversation

vsemozhetbyt
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt commented Jun 16, 2017 •
edited
Loading

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

benchmark, readline, repl, url, util

Use non-capturing grouping or remove capturing completely when:

  • capturing is useless per se, e.g. in test() check;
  • captured groups are not used afterward at all;
  • some of the later captured groups are not used afterward.

@nodejs-github-bot nodejs-github-bot added the benchmark Issues and PRs related to the benchmark subsystem. label Jun 16, 2017
@mscdex mscdex mentioned this pull request Jun 16, 2017
Closed
2 tasks
@vsemozhetbyt
readline,repl,url,util: remove needless capturing
a08e86b 
Use non-capturing grouping or remove capturing completely when:

* capturing is useless per se, e.g. in test() check;
* captured groups are not used afterwards at all;
* some of the later captured groups are not used afterwards.
@vsemozhetbyt vsemozhetbyt changed the title benchmark: remove needless RegExp capturing benchmark, libs: remove needless RegExp capturing Jun 16, 2017
@vsemozhetbyt
Copy link
Contributor Author

#13720 is added as a new commit here according to the request.

@addaleax @cjihrig @jasnell You may consider recalling your reviews for now.

@refack refack added the lib / src Issues and PRs related to general changes in the lib or src directory. label Jun 16, 2017
@vsemozhetbyt
Copy link
Contributor Author

CI was green for #13720. Run another to be on the safe side:

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

@vsemozhetbyt vsemozhetbyt added readline Issues and PRs related to the built-in readline module. repl Issues and PRs related to the REPL subsystem. url Issues and PRs related to the legacy built-in url module. util Issues and PRs related to the built-in util module. labels Jun 16, 2017
@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Jun 16, 2017 •
edited
Loading

If I recall correctly we use lib/src label for C++ source only)

@vsemozhetbyt vsemozhetbyt removed the lib / src Issues and PRs related to general changes in the lib or src directory. label Jun 16, 2017
vsemozhetbyt added a commit that referenced this pull request Jun 19, 2017
@vsemozhetbyt
benchmark: remove needless RegExp capturing
b945771 
Use non-capturing grouping or remove capturing completely when:

* capturing is useless per se, e.g. in test() check;
* captured groups are not used afterwards at all;
* some of the later captured groups are not used afterwards.

PR-URL: #13718
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
vsemozhetbyt added a commit that referenced this pull request Jun 19, 2017
@vsemozhetbyt
readline,repl,url,util: remove needless capturing
8789904 
Use non-capturing grouping or remove capturing completely when:

* capturing is useless per se, e.g. in test() check;
* captured groups are not used afterwards at all;
* some of the later captured groups are not used afterwards.

PR-URL: #13718
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@vsemozhetbyt
Copy link
Contributor Author

Landed in bed3579...8789904

@vsemozhetbyt vsemozhetbyt deleted the benchmark-no-needless-capture branch June 19, 2017 15:13
addaleax pushed a commit that referenced this pull request Jun 20, 2017
@vsemozhetbyt @addaleax
benchmark: remove needless RegExp capturing
38253af 
Use non-capturing grouping or remove capturing completely when:

* capturing is useless per se, e.g. in test() check;
* captured groups are not used afterwards at all;
* some of the later captured groups are not used afterwards.

PR-URL: #13718
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax pushed a commit that referenced this pull request Jun 20, 2017
@vsemozhetbyt @addaleax
readline,repl,url,util: remove needless capturing
7b03d68 
Use non-capturing grouping or remove capturing completely when:

* capturing is useless per se, e.g. in test() check;
* captured groups are not used afterwards at all;
* some of the later captured groups are not used afterwards.

PR-URL: #13718
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@addaleax addaleax mentioned this pull request Jun 21, 2017
Merged
addaleax pushed a commit that referenced this pull request Jun 21, 2017
@vsemozhetbyt @addaleax
benchmark: remove needless RegExp capturing
11f4562 
Use non-capturing grouping or remove capturing completely when:

* capturing is useless per se, e.g. in test() check;
* captured groups are not used afterwards at all;
* some of the later captured groups are not used afterwards.

PR-URL: #13718
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax pushed a commit that referenced this pull request Jun 21, 2017
@vsemozhetbyt @addaleax
readline,repl,url,util: remove needless capturing
e786926 
Use non-capturing grouping or remove capturing completely when:

* capturing is useless per se, e.g. in test() check;
* captured groups are not used afterwards at all;
* some of the later captured groups are not used afterwards.

PR-URL: #13718
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@addaleax addaleax mentioned this pull request Jun 21, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
benchmark Issues and PRs related to the benchmark subsystem. readline Issues and PRs related to the built-in readline module. repl Issues and PRs related to the REPL subsystem. url Issues and PRs related to the legacy built-in url module. 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.
8 participants
@vsemozhetbyt @refack @jasnell @addaleax @lpinca @cjihrig @MylesBorins @nodejs-github-bot