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: vcbuild refactoring #17299

Merged
merged 1 commit into from
Dec 2, 2017
Merged

Conversation

refack
Copy link
Contributor

@refack refack commented Nov 25, 2017

  • More consistent compilation of arguments to ./configure
  • Code indent
  • Call to explicit .exe binary
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build,windows

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Nov 25, 2017
@refack
Copy link
Contributor Author

refack commented Nov 25, 2017

@refack
Copy link
Contributor Author

refack commented Nov 25, 2017

/CC @nodejs/build @nodejs/platform-windows

Copy link
Member

@tniessen tniessen left a comment

Choose a reason for hiding this comment

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

Basically LGTM, did you test all options (just to be sure)?

@refack
Copy link
Contributor Author

refack commented Dec 2, 2017

I tested this by mocking configure to simply print the args and exit(1). Made sure the calls are the same.

PR-URL: nodejs#17299
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@refack refack force-pushed the vcbuild-refactoring branch from 400f69a to 9fb390a Compare December 2, 2017 23:02
@refack refack merged commit 9fb390a into nodejs:master Dec 2, 2017
@refack refack deleted the vcbuild-refactoring branch December 5, 2017 13:53
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
PR-URL: #17299
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
PR-URL: #17299
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
PR-URL: #17299
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
PR-URL: #17299
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
rvagg added a commit to rvagg/io.js that referenced this pull request Jan 9, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: nodejs#17299
Ref: nodejs/abi-stable-node#289
mhdawson pushed a commit that referenced this pull request Jan 11, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: #17299
Ref: nodejs/abi-stable-node#289

PR-URL: #18031
Ref: #17299
Ref: nodejs/abi-stable-node#289
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
kfarnung pushed a commit to kfarnung/node-chakracore that referenced this pull request Jan 11, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: nodejs/node#17299
Ref: nodejs/abi-stable-node#289

PR-URL: nodejs/node#18031
Ref: nodejs/node#17299
Ref: nodejs/abi-stable-node#289
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
evanlucas pushed a commit that referenced this pull request Jan 30, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: #17299
Ref: nodejs/abi-stable-node#289

PR-URL: #18031
Ref: #17299
Ref: nodejs/abi-stable-node#289
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
MylesBorins pushed a commit that referenced this pull request May 15, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: #17299
Ref: nodejs/abi-stable-node#289

PR-URL: #18031
Ref: #17299
Ref: nodejs/abi-stable-node#289
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
MylesBorins pushed a commit that referenced this pull request May 15, 2018
--tag needs to be set after `getnodeversion` because TAG is defined in
there when DISTTYPE is not "release", setting it before `getnodeversion`
leads to --tag not being passed down in to `configure` and
src/node_version.h setting it as `-pre` by default. This change restores
the functionality that properly sets the TAG for nightlies, rc builds
and other custom build types.

Ref: #17299
Ref: nodejs/abi-stable-node#289

PR-URL: #18031
Ref: #17299
Ref: nodejs/abi-stable-node#289
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
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. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants