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

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

Closed
tunnckoCore opened this issue Nov 14, 2017 · 6 comments
Closed

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

tunnckoCore opened this issue Nov 14, 2017 · 6 comments

Comments

@tunnckoCore
Copy link
Member

tunnckoCore commented Nov 14, 2017

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
Copy link
Member Author

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

tunnckoCore pushed a commit that referenced this issue Nov 14, 2017
Signed-off-by: Charlike Mike Reagent <olsten.larck@gmail.com>
@ghost
Copy link

ghost commented Dec 1, 2017

@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
Copy link
Member Author

tunnckoCore commented Dec 1, 2017

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
Copy link

ghost commented Dec 1, 2017

@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
Copy link
Member Author

Yup, it's in the roadmap.

@tunnckoCore
Copy link
Member Author

tunnckoCore commented Feb 12, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant