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

test: fix zlib version regex by lpinca · Pull Request #48227 · nodejs/node · GitHub

/ node Public

test: fix zlib version regex - #48227

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
lpinca:fix/version-regex
May 30, 2023
ConversationCommits (1)ChecksFiles changed
Merged

test: fix zlib version regex#48227
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
lpinca:fix/version-regex

Conversation

lpinca commented May 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Add support for subrevision in the regular expression for the zlib version.

Refs: https://github.com/madler/zlib/blob/48c3741002ac/zlib.h#L40

nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels May 28, 2023
lpinca force-pushed the fix/version-regex branch from dce2065 to 641b4db Compare May 28, 2023 06:05
lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label May 28, 2023

lpinca commented May 28, 2023

Copy link
Copy Markdown
Member Author

Fixes CI for #48218.

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 28, 2023

nodejs-github-bot commented May 28, 2023

Copy link
Copy Markdown
Collaborator

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

Mesteery approved these changes May 28, 2023
View reviewed changes
Comment thread test/parallel/test-process-versions.js Outdated

Mesteery May 28, 2023

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
Suggested change
assert.match(process.versions.zlib, /^\d+\.\d+\.\d+(?:\.\d+)?(?:-.*)?$/);
assert.match(process.versions.zlib, /^\d+(?:\.\d+){2,3}(?:-.*)?$/);

lpinca May 28, 2023

Copy link
Copy Markdown
Member Author

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

For consistency I would prefer to keep it as is, see line 72. I'm ok with a a new PR to shorten both regex.

Mesteery May 28, 2023

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

For me, the reason the first regex wasn't shortened that way is that it would complicate it more than anything else. Unlike this one, which is already a little longer, and in my opinion my suggestion is just as consistent as this regex. But I understand and I don't mind a new PR.

lpinca May 28, 2023

Copy link
Copy Markdown
Member Author

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'm talking about this one

node/test/parallel/test-process-versions.js

Line 72 in d81151e

/^\d+\.\d+\.\d+(?:\.\d+)?-node\.\d+(?: \(candidate\))?$/
but I have no strong opinion. I will apply your suggestion.

Mesteery May 28, 2023

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

Oh, you're completly right, sorry for my misunderstanding.

test: fix zlib version regex
43c332f 
Add support for subrevision in the regular expression for the zlib
version.

Refs: https://github.com/madler/zlib/blob/48c3741002ac/zlib.h#L40
lpinca force-pushed the fix/version-regex branch from 641b4db to 43c332f Compare May 28, 2023 18:32
lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label May 28, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 28, 2023

nodejs-github-bot commented May 28, 2023

Copy link
Copy Markdown
Collaborator

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

github-actions Bot mentioned this pull request May 29, 2023
Open
22 tasks
lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label May 29, 2023
github-actions Bot mentioned this pull request May 30, 2023
Open
23 tasks
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 30, 2023
nodejs-github-bot merged commit 6a6b3c5 into nodejs:main May 30, 2023

nodejs-github-bot commented May 30, 2023

Copy link
Copy Markdown
Collaborator

Landed in 6a6b3c5

targos pushed a commit that referenced this pull request May 30, 2023
test: fix zlib version regex
215b2bc 
Add support for subrevision in the regular expression for the zlib
version.

Refs: https://github.com/madler/zlib/blob/48c3741002ac/zlib.h#L40
PR-URL: #48227
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
lpinca deleted the fix/version-regex branch May 30, 2023 13:31
This was referenced May 31, 2023
Open
Open
Open
Open
targos mentioned this pull request Jun 4, 2023
Merged
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
test: fix zlib version regex
5667bb9 
Add support for subrevision in the regular expression for the zlib
version.

Refs: https://github.com/madler/zlib/blob/48c3741002ac/zlib.h#L40
PR-URL: #48227
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
test: fix zlib version regex
0e3312b 
Add support for subrevision in the regular expression for the zlib
version.

Refs: https://github.com/madler/zlib/blob/48c3741002ac/zlib.h#L40
PR-URL: nodejs#48227
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams mentioned this pull request Jul 10, 2023
Merged
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
test: fix zlib version regex
2957a81 
Add support for subrevision in the regular expression for the zlib
version.

Refs: https://github.com/madler/zlib/blob/48c3741002ac/zlib.h#L40
PR-URL: nodejs#48227
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
test: fix zlib version regex
3d51acc 
Add support for subrevision in the regular expression for the zlib
version.

Refs: https://github.com/madler/zlib/blob/48c3741002ac/zlib.h#L40
PR-URL: nodejs#48227
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
+1 more reviewer
Mesteery Mesteery approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL