search

scala-steward-org / scala-steward

:robot: A bot that helps you keep your projects up-to-date
Apache License 2.0
1.15k stars 496 forks source link

Only return a failure exit code when all repos have failed #3369

Closed ioannakok closed 3 months ago

ioannakok commented 4 months ago

When even one repository fails in a scala steward run the workspace is not persisted by GitHub Actions because GH Actions won't persist the workspace of a failing job. Persisting workspace is crucial for respecting pullRequests.frequency configuration because the workspace is how scala steward records that it has opened a pull request. This means that one failing repository can cause a lot of user annoyance because their configuration is no longer respected and they get too many PRs opened. This change aims to fix that by only returning a failure code if all repos have failed. So long as at least one repository succeeded the exit code will be success and the workspace will persist. If administrators need to know what repos are failing they can use the jobs summary introduced by: https://github.com/scala-steward-org/scala-steward/pull/3071

See more here: https://github.com/guardian/scala-steward-public-repos/issues/60

cc @alejandrohdezma

rtyley commented 4 months ago

@mzuehlke how does this look to you? To us at the Guardian this does seems like the best fix to the problem?

One possible alternative would be to modify the behaviour of scala-steward-action so that workspace.saveWorkspaceCache() gets invoked even if coursier.launch() throws an exception, though that probably runs the risk of persisting some inconsistent state.

alejandrohdezma commented 4 months ago

Hey @ioannakok, thanks for this! I think this could make sense for a lot of use cases, but not for others. Maybe we can expand on it and put it behind a flag that's been sent in the params. That way we can propagate that flag from the action and people can decide whether to fail if everything is failing or not. What do you think?

rtyley commented 3 months ago

I think this could make sense for a lot of use cases, but not for others. Maybe we can expand on it and put it behind a flag that's been sent in the params. That way we can propagate that flag from the action and people can decide whether to fail if everything is failing or not.

Thanks @alejandrohdezma - I've added a flag with 0607cfc93e0a79683de7bb7ad61fa99a6502c915, does that look ok?

alejandrohdezma commented 3 months ago

Great work! Thanks! ❤️