fastify / workflows

Reusable workflows for use in the Fastify organization
MIT License
9 stars 6 forks source link

fail-fast input setting #87

Open bcomnes opened 1 year ago

bcomnes commented 1 year ago

When trying to debug CI errors in the matrix, its really frustrating to have to uncover them randomly one at a time. Can we change this so that all environments run to completion so its easier to find where problems live?

Checklist

Uzlopak commented 1 year ago

You want to enforce this in fastify org? Or do you use our workflows for your repos, too?

bcomnes commented 1 year ago

I'm trying to fix CI on fastify-cli and an its really hard to tell which issue is a test problem and which is an environment problem since they keep failing in different orders. If they all ran to completion, it would be easier to determine this.

Perhaps this could be at a minimum, an input to the workflow so I could turn it on while debugging if fail-fast is actually the desired default.

https://github.com/fastify/fastify-cli/pull/639

Uzlopak commented 1 year ago

Yes an input variable would be desired.

bcomnes commented 1 year ago

Ok added a fail-fast input consistently across workflows that accept inputs.

bcomnes commented 1 year ago

Thanks for the PR!

Could you please fix the misspelling of 'strategy' in the descriptions added, and add the setting to the inputs table in the readme (alphabetically sorted)?

Done.

Uzlopak commented 1 year ago

Is the workflow validation a false positive?

bcomnes commented 1 year ago

Looks like its a boolean/string type coersion issue? Any advice? I tried following a similar pattern I saw elsewhere but I think if statements are special cases in actions.

bcomnes commented 1 year ago

I dont understand, is it because the input is getting passed in as an expression that the validator is treating it not as a boolean? Is there a way to tell the validator that it's definitely a boolean?

bcomnes commented 1 year ago

Sorry I had to drop off this yak shave the other week.

Since it looks like there is a strange incompatibility between input type coercion and the validator tool here (happy to try and track that down and bring upstream if you want, please let me know if you do!), and I was able to isolate out the issues in the issues that prompted this PR, I have the following proposal:

Can anyone name a scenario where fail-fast: true is actually desired in the fastify org? The only case I can think of is if there were private repos burning paid minutes (which isn't a problem typically for open source projects). Otherwise, it seems like fail-fast: false should just be the default everywhere.

If folks agree, can we change it to that now, while we work through the upstream validator bugs?

Fdawgs commented 1 year ago

@bcomnes could you rebase this PR please? Want to see if #88 has resolved this.

If not, tempted to just tear that action out. It's v0.x.x so not massively stable yet.

voxpelli commented 1 year ago

Can anyone name a scenario where fail-fast: true is actually desired in the fastify org? The only case I can think of is if there were private repos burning paid minutes (which isn't a problem typically for open source projects). Otherwise, it seems like fail-fast: false should just be the default everywhere.

I agree that in most cases fail-fast: false should be the desirable? (And it would be good to document why its not if there's a good reason for it, that way the likes of me won't comment on it)

bcomnes commented 1 year ago

I'll rebase the next time I'm at the keys.

Uzlopak commented 1 year ago

I wonder why we dont have here the option to simply merge the default branch into this PR, as we can do it in fastify core repo.

bcomnes commented 1 year ago

Rebased.