FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

benchmark: simplify http benchmarker regular expression by Trott · Pull Request #38206 · nodejs/node · GitHub

/ node Public

benchmark: simplify http benchmarker regular expression - #38206

Merged
Trott merged 1 commit into
nodejs:masterfrom
Trott:non-capture
Jan 12, 2022
Merged

benchmark: simplify http benchmarker regular expression#38206
Trott merged 1 commit into
nodejs:masterfrom
Trott:non-capture

Conversation

Trott commented Apr 11, 2021

Copy link
Copy Markdown
Member

A non-capturing group inside a capturing group has no effect. Simplify
the regular expression.

nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. http Issues or PRs related to the http subsystem. labels Apr 11, 2021

Copy link
Copy Markdown
Collaborator

Comment thread benchmark/_http-benchmarkers.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Maybe the original intent was to handle integer values? I don't know if that's necessary, please disregard if we know that doesn't happen.

Suggested change
const rex = /(\d+\.\d+) req\/s/;
const rex = /(\d+(?:\.\d+)?) req\/s/;

ErickWendel Jan 10, 2022
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think it shouldn't work with integers. The h2load output is something like "finished in 233.18s, 0.43 req/s, 105B/s" his docs

Trott added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 12, 2022
A non-capturing group inside a capturing group has no effect. Simplify
the regular expression.

PR-URL: nodejs#38206
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>

Trott commented Jan 12, 2022

Copy link
Copy Markdown
Member Author

Landed in 36035e0

Trott merged commit 36035e0 into nodejs:master Jan 12, 2022
Trott deleted the non-capture branch January 12, 2022 06:58
targos pushed a commit that referenced this pull request Jan 14, 2022
A non-capturing group inside a capturing group has no effect. Simplify
the regular expression.

PR-URL: #38206
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
A non-capturing group inside a capturing group has no effect. Simplify
the regular expression.

PR-URL: #38206
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
danielleadams mentioned this pull request Feb 1, 2022
panva removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 27, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark Issues and PRs related to the benchmark subsystem. http Issues or PRs related to the http subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL