Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Add support for node v6 #8884

Merged
merged 7 commits into from
May 4, 2017
Merged

Add support for node v6 #8884

merged 7 commits into from
May 4, 2017

Conversation

bsudekum
Copy link

@bsudekum bsudekum commented May 3, 2017

It's time to add support for node v6. I think this should be fairly straightforward quest:

  • Verify no dependencies need to be updated for v6
  • Update test matrix to begin testing both node v4 and v6
  • Merge this
  • Add new release with v6 binaries

/cc @tmpsantos @jfirebaugh @springmeyer

export PUBLISH=true
make test-node
./platform/node/scripts/after_success.sh
- slack: *slack
Copy link
Author

Choose a reason for hiding this comment

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

Unsure if this is the best way to add another version for bitrise to test.

@springmeyer
Copy link
Contributor

@bsudekum looks like you are using {configuration} in the package.json but not passing --debug to node-pre-gyp when publishing. To avoid builds clobbering each other this is important. This is done in #8865. Perhaps you should test and merge #8865 first before adding this? Or focus just on node v6 and hit debug builds later.

@bsudekum
Copy link
Author

bsudekum commented May 4, 2017

The last todo is to figure out how to test and publish macos on node v6. Unfortunately bitrise does not support matrix builds like on travis. We have a few options here:

  • Add more to the current bitrise workflow as done in 2db9a9c. This now unlinks node v4, links node v6 and then run tests again.
  • Add a new bitrise app specifically for node v6
  • Something else?

/cc @1ec5

@1ec5 1ec5 added build Node.js node-mapbox-gl-native labels May 4, 2017
Bobby Sudekum added 2 commits May 4, 2017 08:18
@bsudekum
Copy link
Author

bsudekum commented May 4, 2017

Now that Node macOS is running tests for both v4 & v6, the total test time is increased by ~5m. @jfirebaugh how do you feel about this? What if we support v4 & v6 only for a few months to give developers a grace period to upgrade?

Copy link
Contributor

@jfirebaugh jfirebaugh left a comment

Choose a reason for hiding this comment

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

Yeah, providing we a plan to eventually deprecate node v4 support, I think this is reasonable. There will have to be some window where we support both.

brew unlink node@4
brew link --overwrite node@6 --force
make clean
make test-node
./platform/node/scripts/after_success.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

after_success.sh is what performs the publish, so it needs to be run after both the node@4 and node@6 builds or otherwise only the latter will get published. Suggest something like:

make test-node && ./platform/node/scripts/after_success.sh

.travis.yml Outdated
@@ -133,6 +133,58 @@ matrix:
after_failure:
- aws s3 cp . s3://mapbox/mapbox-gl-native/render-tests/$TRAVIS_JOB_NUMBER --recursive --exclude "*" --include "*.trace"

# EGL - Node v6 - Clang 3.9 - Debug
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless there's a compelling reason to test the Node v6 debug build independently from Node v4 debug and Node v6 release, we can probably omit this row from the matrix.

@bsudekum bsudekum merged commit 36ec203 into master May 4, 2017
@bsudekum bsudekum deleted the node-v6 branch May 4, 2017 17:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
build Node.js node-mapbox-gl-native
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants