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

build: Simplify fetching release version #1405

Closed

Conversation

jbergstroem
Copy link
Member

Avoid calling python for something we can do in grep/sed (we're already avid users of said tools).

@brendanashworth brendanashworth added the build Issues and PRs related to build files or the CI. label Apr 13, 2015
@@ -197,7 +197,7 @@ docclean:
RAWVER=$(shell $(PYTHON) tools/getnodeversion.py)
VERSION=v$(RAWVER)
FULLVERSION=$(VERSION)
RELEASE=$(shell $(PYTHON) tools/getnodeisrelease.py)
RELEASE=($shell grep "#define NODE_VERSION_IS_RELEASE" src/node_version.h | sed 's/[^0-9]//g')
Copy link
Member

Choose a reason for hiding this comment

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

This can be shortened to sed -ne 's/#define NODE_VERSION_IS_RELEASE \([01]\)/\1/p' src/node_version.h

@bnoordhuis
Copy link
Member

LGTM with a suggestion.

@rvagg
Copy link
Member

rvagg commented Apr 13, 2015

lgtm

jbergstroem added a commit that referenced this pull request Apr 13, 2015
PR-URL: #1405
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@jbergstroem
Copy link
Member Author

Fixed in f9a2d31. Thanks for the review.

@rvagg rvagg mentioned this pull request Apr 14, 2015
rvagg added a commit to rvagg/io.js that referenced this pull request Apr 14, 2015
rvagg added a commit that referenced this pull request Apr 14, 2015
fixes broken 1.7.0 build, unreviewed quick patch

Ref: #1405
PR-URL: #1421
rvagg added a commit that referenced this pull request Apr 14, 2015
fixes broken 1.7.0 build, unreviewed quick patch

Ref: #1405
PR-URL: #1421
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants