lindell / multi-gitter

Update multiple repositories in with one command
Apache License 2.0
819 stars 64 forks source link

Feature request: flag to return return a failure exit code #474

Open nwsparks opened 2 months ago

nwsparks commented 2 months ago

The feature request

flag to return return a failure exit code if any repo(s) do not succeed.

apologies if I missed that this is currently available somewhere, did not see it.

My use case

Would like to use multi-gitter with some cron jobs for package updating and would like a clean way of notifying when it fails on a repo.

Implementation

jcmcken commented 2 weeks ago

This also makes it more difficult to run in a CI context, where a failure should indicate to the CI system that the operation was not successful. That's usually signaled with the exit code

lindell commented 2 weeks ago

This is a good idea. However, the option would also have to define what the user defines as an error.

Some things that could be considered an error:

Not all of these might be considered an error by all users.

nwsparks commented 2 weeks ago

I would keep it simple and just deal with execution errors of any kind. I think that covers the vast majority of use cases. You'll get lost in the weeds otherwise.

I don't think skips, no changes should be considered for this. Things like no change could be dealt with in the script by detecting it and throwing an error if people really want that behavior.

If you felt it was really necessary to get so granular you could start with a flag for execution errors and build on with more flags later for the other scenarios?

lindell commented 2 weeks ago

@nwsparks It might be the most common use case, but just look at the reference from another comment @jcmcken made 3 days ago. That seems to have been about skips not causing errors.

nwsparks commented 2 weeks ago

Hmm, I reviewed both comments and didn't see a specific mention about skipping. I think your point is a good one, it seems like a separate issue though that would be handled by additional flags or redesign of existing flags such as --conflict-strategy error

jcmcken commented 2 weeks ago

From my point of view, it makes sense to error in cases where doing the same operation by hand in a shell would have a non-zero exit code (even though this tool may not be using any CLIs directly). After all, kind of the point of multi-gitter (as I see it at least) is to save you from having to do these operations by hand.

So to me, that includes everything in your list

nwsparks commented 2 weeks ago

Thanks for the discussion and sorry if I'm misunderstanding, it sounds like you are saying all of those would be part of this new feature? I agree on script failure and git push/pull failure.

branch already exists

Aren't you already dealing with this via --conflict-strategy? If someone wants exit codes and also --conflict-strategy replace how would that work? I think it becomes confusing and unclear if you include it here. It would make more sense to expand --conflict-strategy if additional behavior for existing branches is desired. I don't think this should be added as part of this feature request.

empty changes

The use case for adding exit codes would primarily be for automated execution, in our case we're using the tool to do nodejs package updates weekly across 50+ repositories. Executions that result in no changes are not an error, it just means that no updates were found. If you make no changes result in an error then we are no better off than we were before and would get no benefit out of this new feature. I suspect that this would be the case for many who would use this feature.

I think again if this behavior is desired (fail on empty changes) it needs to be a separate flag and unrelated to the feature request here.

jcmcken commented 2 weeks ago

@nwsparks

Some of it is up for interpretation I think. I'm just trying to give a backing theory for how to design the exit codes

For replace conflict strategy, I think it's straightforward. If you force push a branch with git CLI (git push -f) where you have conflicts between local and remote, it should exit with a 0 exit code, with remote getting deleted and replaced by local.

Normally, if you use plain git push (no -f), it should error since the remote and local have diverged. But since the conflict strategy skip already implies a specific behavior (continue without erroring), that should not change, so the exit code would be 0. (I'm not advocating for any backwards incompatible change). In this case, the CLI analogue (how you could accomplish this behavior just using the git CLI and shell commands) is more like you're running git ls-remote to check if the remote branch exists already, and if it does, continuing without action. Then maybe it makes sense to have another conflict strategy called error as you stated

The issue I want to address with branches is when there are many developers altering the same GitOps repos. If you're modifying 50 or 100 repos, and 1 is skipped because someone else has already created the named branch, it's likely that you will miss this happened unless you're carefully observing the output. In cases like this, an exit code is a single, unifying signal that tells the user they need to carefully read the logs and take some action. Otherwise the user needs to know the specific content of log messages, use things like grep, and so on -- a more brittle method of error detection