semantic-release / condition-travis

:no_entry_sign: semantic-release plugin to check Travis CI environment before publishing.
MIT License
9 stars 11 forks source link

Make it work out of the box with Travis build stages #96

Closed felixfbecker closed 6 years ago

felixfbecker commented 6 years ago

Right now I have to override verifyConditions to not include condition-travis to make it work. That adds a lot of friction in setting semantic-release up. I wonder if there is a way to check if build stages are used and if yes, don't check for the other builds to pass?

pvdlg commented 6 years ago

There is: see condition-travis#using-travis-build-stages.

You just have to set BUILD_LEADER_ID.

felixfbecker commented 6 years ago

I get that, but I'm saying it's a suboptimal experience, especially for new users of semantic-release. I assume that going forward, people will use release stages over travis-deploy-once, and I wanted to discuss ways how to improve that.

It's also easy to forget to change BUILD_LEADER_ID when adding something to the test matrix. And if you want to run semantic-release in script instead of after_success (which imo you should because after_success swallows errors) then you have to disable condition-travis completely or it will fail. Where "disabling" means you have to override verifyCondition to repeat all the other plugins you do want to run, like @semantic-release/github, @semantic-release/npm.

pvdlg commented 6 years ago

There is a similar discussion here: semantic-release/semantic-release#486

Another thing to consider with Build Stages is that we have no way in semantic-release to verify that the configuration of Build Stages is done properly. A misconfiguration in the build stages might trigger an unwanted release. The amount of safety check we should put in the code to prevent problems due to misconfiguration is arguable, but a lot of issue we have are related to a misconfiguration that have created a release or a tag and the user struggles to recover from that. In other type of software, I'd say that failures due to misconfiguration are completely acceptable and expected. In the case of semantic-release it result in an unwanted release which can have a lot of consequences:

  • Some user might upgrade to the new release before you relaize the problem happened
  • Tools like greenkeeper will be triggered, creating update PR, that might get merge
  • Any other automated tool that listening to npm publishes will be triggered (blog posts, mails, tweet, confetis, etc...)
  • Unpublishing the version on npm prevent to reuse the version later (if you release 2.0.0 and unpublish it, you won't be able to release 2.0.0 it again, ever.You'd have to take manual step to release 2.0.1 or 3.0.0)
  • All that concerns mostly big projects. But still, it's important for semantic-release to take extra care in checking everything possible before making the release.

We want to do a hard check on the release happening only if the tests of all jobs running on the CI. That's not possible by running semantic-release on the script step.

The Travis API v3 allow to get some info about the build Stages, but I didn't find a way to determines with certainty which job is supposed to run semantic-release. So we can't guarantee that the release won't happen on the wrong job, or on multiple jobs.

The typical scenario would be to have a test Stage that run the tests on different Node version and OS and a release Stage that runs semantic-release. But there might be a lot of other situation to handle: -The jobs in the test Stage also run semantic-release => We need to prevent the release.

The BUILD_LEADER_ID is not ideal but it forces the user to read the doc and think about what they are configuring, rather than doing an unexpected release without the user "consent".

I spent quite some time trying to figure out a better solution, but I couldn't come up with something satisfactory so far...My conclusion is that relying on checks in the CI is cumbersome and necessarily complex. And no matter the solution there will still be the problem of projects running multiple CIs platforms. So it's probably better to solve this issue by not running semantic-release on the CI anymore and create a Github app that monitor the CI status on the commits and trigger a custom CI build that run semantic-release. That comes with several challenges as well, but it would solve more problems I think.

That said, if you can propose a Build Stage based solution that fulfill the needs mentioned above, I will happily look at it :)

felixfbecker commented 6 years ago

Your points are all valid. I'll have to think about this a bit more, but making some requirements like forcing the stage to be named "release" and doing harder checks could get us there. It should also be possible to validate to config locally before running the build, because it doesn't depend on any runtime things. If we make the CLI spit out guaranteed-to-work config for many use cases we can also eliminate risk. And then, maybe we can disable the check for travis-deploy-once if build stages are detected in a fuzzy way safely.

So it's probably better to solve this issue by not running semantic-release on the CI anymore and create a Github app that monitor the CI status on the commits and trigger a custom CI build that run semantic-release. That comes with several challenges as well, but it would solve more problems I think.

It's not clear to me how the GitHub app would solve all this. Is there a thread with more details that I can join? It feels a bit off-topic here.

pvdlg commented 6 years ago

but making some requirements like forcing the stage to be named "release" and doing harder checks could get us there.

Yes but I'm not sure it would be that much of an improvement in terms of convenience compared to using BUILD_LEADER_ID.

It should also be possible to validate to config locally before running the build, because it doesn't depend on any runtime things. If we make the CLI spit out guaranteed-to-work config for many use cases we can also eliminate risk.

Not really...The CLI might create a valid config when a user setup semantic-release but nothing prevent further modification of the travis.yml.

It's not clear to me how the GitHub app would solve all this. Is there a thread with more details that I can join? It feels a bit off-topic here.

There is a couple of mentions here and there in the repo issues. Nothing is set in stone, far from that. Once we'll have something to propose we'll open an issue to get feedback from the community. The first task for me is to get semantic-release in it "current CI iteration" up to speed, stable, maintainer friendly and add important features (language agnostic, multiple branches/dist channel/pre-release). It turns out to be a bigger task than I though :) I released a lot of features recently, but the documentation is lagging behind...So at the moment, regarding my time investment, writing documentation is a higher priority.

pvdlg commented 6 years ago

And other thing to consider is that Build Stages are in beta version and can change at any time. In addition it's not supported on Travis Pro.

Maybe it would be simple and helpful enough to check if there is a Build Stage name release or semantic-release limit the jobs considered by travis-deploy-once to the ones within this build stage? This way if you have a Build Stage named release with one job then travis-deploy-once will automatically consider it as the build leader. If you have multiple jobs configured in the release Stage (because you do different type of release for some reason) then travis-deploy-once will use the same algorithm but limit to those jobs, so the highest Node version within the Build Stage release will be used.

We would have to modify travis-deploy-once to accept a new argument stages that will be a list of the possible Build Stages to consider. Then travis-deploy-once will consider the first one in the list that exist in the current Travis Build. Then conditions-travis would call travis-deploy-once passing ['semantic-release', 'release'].

felixfbecker commented 6 years ago

Yeah that sounds good to me, and I think it's reasonable to default it to release if we document that.

gr2m commented 6 years ago

I’d like to wait for this until stages are out of beta, because things might still change. It adds quite a lot of complexity based on a lot of assumptions right now. Do you think there is any way to get data to back "I assume that going forward, people will use release stages"? Maybe you could ping Travis if they have any data how the adoption is going of build stages in node projects?

felixfbecker commented 6 years ago

Sure they are in beta, but so is semantic-release ^10 - for using semantic-release for non-npm projects, semantic-release ^10 and build stages are the best way. Eventually both will come out of beta.

Maybe you could ping Travis if they have any data how the adoption is going of build stages in node projects?

Maybe @Lyoness knows?

pvdlg commented 6 years ago

@felixfbecker would #30 be an acceptable solution for you?

It would work transparently (without BUILD_LEADER_ID) as long as:

So you can just create a Build Stage that runs last and set it's node version to the highest in your build, or even better you can set it to node_js: node so it will always run the latest stable version and will always be considered the highest.

felixfbecker commented 6 years ago

I think #30 is unrelated?

pvdlg commented 6 years ago

Oops, I meant semantic-release/travis-deploy-once#30