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.

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

lib: fix version check in tick processor #16769

Merged
merged 3 commits into from
Nov 7, 2017
Merged

Conversation

bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Nov 5, 2017

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Nov 5, 2017
Introduced in 70832bc ("build: add V8 embedder version string".)

Fixes: nodejs#16736
PR-URL: nodejs#16769
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Make it possible to test the versionCheck() function from that file in
isolation.

PR-URL: nodejs#16769
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Verify that v8-version log lines are parsed and matched correctly.

Fixes: nodejs#16736
PR-URL: nodejs#16769
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis bnoordhuis closed this Nov 7, 2017
@bnoordhuis bnoordhuis deleted the fix16736 branch November 7, 2017 11:17
@bnoordhuis bnoordhuis merged commit 698bb96 into nodejs:master Nov 7, 2017
cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 7, 2017
Introduced in 70832bc ("build: add V8 embedder version string".)

Fixes: nodejs#16736
PR-URL: nodejs#16769
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 7, 2017
Make it possible to test the versionCheck() function from that file in
isolation.

PR-URL: nodejs#16769
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 7, 2017
Verify that v8-version log lines are parsed and matched correctly.

Fixes: nodejs#16736
PR-URL: nodejs#16769
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig cjihrig mentioned this pull request Nov 7, 2017
@MylesBorins
Copy link
Contributor

Should this be backported to v6.x-staging or v8.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

@bnoordhuis
Copy link
Member Author

Since this fixes a bug introduced in #15785 and that PR is marked semver-major, I'm guessing no. I'll update the labels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants