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.

Sign up for GitHub

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

Jump to bottom

build: move available-node variable to top #22356

Closed
wants to merge 1 commit into from
Closed

build: move available-node variable to top #22356

wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Aug 16, 2018

Currently node_use_openssl uses the available-node variable before it is
defined causing the conditions that use it before that point to
evaluate incorrectly. As an example running the target
test/addons/.docbuildstamp will currently be skipped:

$ make test/addons/.docbuildstamp
Skipping .docbuildstamp (no crypto)

With this commit the target will only be skipped if configured --without-ssl.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@danbev build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/570/pipeline

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Aug 16, 2018
@danbev
Copy link
Contributor Author

danbev commented Aug 16, 2018

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

@joyeecheung joyeecheung mentioned this pull request Aug 17, 2018
Closed
@danbev
build: move available-node variable to top
44d4075 
Currently node_use_openssl uses the available-node variable before it is
defined causing the conditions that use it before that point to
evaluate incorrectly. As an example running the target
test/addons/.docbuildstamp will currently be skipped:

$ make test/addons/.docbuildstamp
Skipping .docbuildstamp (no crypto)

With this commit the target will only be skipped if configured --without-ssl.
@danbev
Copy link
Contributor Author

danbev commented Aug 20, 2018

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

@danbev
Copy link
Contributor Author

danbev commented Aug 20, 2018

Landed in 28675b6.

@danbev danbev closed this Aug 20, 2018
@danbev danbev deleted the build_move_available_node branch August 20, 2018 16:58
danbev added a commit that referenced this pull request Aug 20, 2018
@danbev
build: move available-node variable to top
28675b6 
Currently node_use_openssl uses the available-node variable before it is
defined causing the conditions that use it before that point to
evaluate incorrectly. As an example running the target
test/addons/.docbuildstamp will currently be skipped:

$ make test/addons/.docbuildstamp
Skipping .docbuildstamp (no crypto)

With this commit the target will only be skipped if configured --without-ssl.

PR-URL: #22356
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Aug 21, 2018
@danbev @targos
build: move available-node variable to top
1f022be 
Currently node_use_openssl uses the available-node variable before it is
defined causing the conditions that use it before that point to
evaluate incorrectly. As an example running the target
test/addons/.docbuildstamp will currently be skipped:

$ make test/addons/.docbuildstamp
Skipping .docbuildstamp (no crypto)

With this commit the target will only be skipped if configured --without-ssl.

PR-URL: #22356
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Sep 3, 2018
@danbev @targos
build: move available-node variable to top
660c515 
Currently node_use_openssl uses the available-node variable before it is
defined causing the conditions that use it before that point to
evaluate incorrectly. As an example running the target
test/addons/.docbuildstamp will currently be skipped:

$ make test/addons/.docbuildstamp
Skipping .docbuildstamp (no crypto)

With this commit the target will only be skipped if configured --without-ssl.

PR-URL: #22356
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request Sep 5, 2018
Merged
This was referenced Sep 6, 2018
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

@richardlau richardlau richardlau approved these changes

@tniessen tniessen tniessen approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@danbev @nodejs-github-bot @jasnell @lpinca @tniessen @richardlau @BridgeAR