standard-release / app

Language independent GitHub App for creating GitHub Releases, following the Conventional Commits and SemVer specifications
https://github.com/apps/standard-release
Apache License 2.0
13 stars 2 forks source link

Should it wait all statuses to finish before release? #10

Closed tunnckoCore closed 5 years ago

tunnckoCore commented 7 years ago

Or it is enough to wait only CI services to end.

Talking about that part

https://github.com/tunnckoCore/semantic-release-app/blob/0bbf1731584d69ab00d5bfc42241c9190e51ec39/src/index.js#L97-L106

I really i lost few hours today before i realized why it does not publish releases, just because of that ;d Initially i was thinking about it and decide that it is good thing to wait everything to finish, but for example bitHound is slow, so publishing release may appear with delay of 5+ minutes in some cases.

/cc @gr2m

tunnckoCore commented 7 years ago

I believe changing it to

  statuses.data.forEach((x) => {
    const isCIStatus = /(?:travis-ci|circleci)/.test(x.context)
    const isPending = x.state === 'pending'
    const isPassing = x.state === 'success'

    if (isPending && isCIStatus && !cache.pending.includes(x.context)) {
      cache.pending.push(x.context)
    }
    if (isPassing && isCIStatus && !cache.passed.includes(x.context)) {
      cache.passed.push(x.context)
    }
  })

would be enough

ghost commented 6 years ago

@olstenlarck I think you would only want to release after status all pass.

In the case of bitHound, if someone submitted a pull request, and the bitHound status came back as failed, I would not accept the pull request, because it failed a requirement. (If bitHound is not required then why have it?)

So, if a status is required (it can fail) then a release should wait on it.

tunnckoCore commented 6 years ago

Yea, but i'm not sure if status object from the api has such thing as "isRequired". Also it's a good thing to do all that in CI. Just add jobs that calls tests and other similiar checks as bitHound on the CI, so we can only check of CI passes. Makes things a lot easier.

Personally i like it that way. Also bitHound and such services may take very long time. Time that i don't have while constantly developing and updating and releasing new versions of my packages. Actually packages that builds that whole thing - detect-new-version, parse-commit-message, new-release, execa-pro and etc.

Once all is much stable, we can switch to check all statuses or add option for that through configuration file.

ghost commented 6 years ago

@olstenlarck maybe consider adding a configuration option to set required status checks?

Renovate does something similar prior to auto-accepting pull requests - https://renovateapp.com/docs/configuration-reference/configuration-options#requiredstatuschecks

tunnckoCore commented 6 years ago

Yup, it's in the roadmap.

tunnckoCore commented 6 years ago

Also... the bitHound - Code status check returns fail status for even easy things like TODO comment.. which is ridiculous to me. Of course i will have them temporary, but.. until then i'm not allowed to release new versions? Damn shit :D So yea, configurable status checks that not includes it by default.

image

the lint issue is actually intentional eslint warning, for console.log statement.