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: make addons build dep. on node_version.h #8861

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

addon tests

Description of change

Make the test/addons/.buildstamp file dependent on src/node_version.h since addons need to be re-compiled after NODE_MODULE_VERSION bumps, e.g. as it happened recently in b5bdff8.

Make the `test/addons/.buildstamp` file dependent on
`src/node_version.h` since addons need to be re-compiled after
`NODE_MODULE_VERSION` bumps, e.g. as it happened recently
in b5bdff8.
@addaleax addaleax added build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests. addons Issues and PRs related to native addons. labels Sep 30, 2016
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Sep 30, 2016
Copy link
Member

@imyller imyller left a comment

Choose a reason for hiding this comment

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

LGTM

@imyller imyller self-assigned this Oct 5, 2016
@imyller
Copy link
Member

imyller commented Oct 5, 2016

@imyller
Copy link
Member

imyller commented Oct 5, 2016

Landing:

  • Approvals (LGTM): 3
  • No objections
  • PR has been open for the minimum time of 48 or 72 hours
  • CI tests passed (only known/unrelated failures)

imyller pushed a commit to imyller/node that referenced this pull request Oct 5, 2016
Make the `test/addons/.buildstamp` file dependent on
`src/node_version.h` since addons need to be re-compiled after
`NODE_MODULE_VERSION` bumps, e.g. as it happened recently
in b5bdff8.

PR-URL: nodejs#8861
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@imyller
Copy link
Member

imyller commented Oct 5, 2016

Landed in 9def097

Thank you, @addaleax

@imyller imyller closed this Oct 5, 2016
@imyller imyller removed their assignment Oct 5, 2016
@addaleax addaleax deleted the addons-node-version-h branch October 5, 2016 22:21
jasnell pushed a commit that referenced this pull request Oct 6, 2016
Make the `test/addons/.buildstamp` file dependent on
`src/node_version.h` since addons need to be re-compiled after
`NODE_MODULE_VERSION` bumps, e.g. as it happened recently
in b5bdff8.

PR-URL: #8861
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
Make the `test/addons/.buildstamp` file dependent on
`src/node_version.h` since addons need to be re-compiled after
`NODE_MODULE_VERSION` bumps, e.g. as it happened recently
in b5bdff8.

PR-URL: #8861
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@MylesBorins
Copy link
Contributor

This is not landing cleanly, @addaleax is this blocked by other PR's being landed first?

@addaleax
Copy link
Member Author

@thealphanerd I am not sure why I labelled this lts-watch in the first place, I don’t think there’s really any point in backporting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addons Issues and PRs related to native addons. build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants