mapbox / node-cpp-skel

Skeleton for bindings to C++ libraries for Node.js using node-addon-api
Creative Commons Zero v1.0 Universal
72 stars 10 forks source link

avoid publishing binaries on git tags #108

Closed mapsam closed 6 years ago

mapsam commented 6 years ago

If you release a new version of a node-cpp-skel library, and use the same commit to build a git tag, it's likely travis will run two jobs and try to build binaries for both jobs. This leads to failing tests based on which jobs can publish binaries first. Workflow:

This isn't hyper critical, but makes tests fail.

cc @springmeyer @GretaCB

mikemorris commented 6 years ago

The workflow I've used for publishing binaries on git tags was:

npm version [patch | minor | major]
git push && git push --tags

This will automatically increment the version in package.json and create a commit with the message 0.3.3 and a v0.3.3 tag pointing to that commit, but only the tag should trigger a binary build (by checking TRAVIS_TAG, CIRCLE_TAG (like in https://github.com/mapbox/mapbox-gl-native/blob/master/platform/node/scripts/after_success.sh#L8) or a CircleCI job filter.

I was not using the [publish binary] commit message hook except when I needed to manually delete and republish a binary for a previously-published version.

springmeyer commented 6 years ago

We could add a check to https://github.com/mapbox/node-cpp-skel/blob/c3d13e76cfb1178c92bf8beca822da4aba8c28f7/scripts/publish.sh#L38 to skip publishing if a tag is detected.

Automatically publishing only on tags as @mikemorris is something to consider. The benefit is avoiding needing to use the [publish binary] commit message. The potential drawbacks are:

sssoleileraaa commented 6 years ago

Thanks for explaining the benefit of having the [publish binary] commit message hook for publishing test binaries as well (used it a couple times while working on a feature branch of vt-shaver-cpp).

I was going to suggest using mason since we're already used to using mason publish but that would add another dependency which isn't so great.

The more @GretaCB and I discussed the different scenarios of this issue the more I like Dane's suggestion, which I believe is saying that we could skip an s3-publish during a tag --push if the latest commit message on master is a [publish binary].

springmeyer commented 6 years ago

Ran into another odd case where a commit attempted to publish when it should not have. The commit message was something like:

Integrate Expressions (#148)

* port to latest mbgl - refs #65

* [publish binary]

And publishing was triggered even though [publish binary] was not in the first line. I suspect it might be the * in the commit message body.

springmeyer commented 6 years ago

landed in #139