-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: fix RELEASE check (URGENT) #1421
Conversation
for reference, the error on a release build is:
I was weary about this change and even went to the effort of testing the suggested |
meh, merging without review |
landed @ fa91b18 |
For what its worth: post-merge LGTM. Sorry about the slip-up. |
still no good, on OSX:
I don't even ... Makefiles :shakesfist: and cross-platform sed :shakesfist: |
Notable changes: * build: A syntax error in the Makefile for release builds caused 1.7.0 to be DOA and unreleased. (Rod Vagg) nodejs#1421.
I can't reproduce. Tried on gnu make 3.81: # cat foo
all:
RELEASE=$(shell sed -ne 's/#define NODE_VERSION_IS_RELEASE \([01]\)/\1/p' src/node_version.h)
# make -f foo
RELEASE=0 |
force-pushed another fix in this from
to
it's now aee86a2 thanks @jbergstroem for spotting that as the problem |
Notable changes: * build: A syntax error in the Makefile for release builds caused 1.7.0 to be DOA and unreleased. (Rod Vagg) #1421
Add allowance for release time commits and error corrections. Per: #48 (comment) Example: nodejs/node#1421 and nodejs/node@fd90b33...50e9fc1.
Add allowance for release time commits and error corrections. Per: nodejs/dev-policy#48 (comment) Example: nodejs/node#1421 and nodejs/node@fd90b33...50e9fc1.
Ref: #1405
please review @jbergstroem || @bnoordhuis asap or I'm just going to merge this and release a 1.7.1, 1.7.0 is a write-off because this is broken unfortunately.