Patrick Steinhardt [off-list ref] writes:
This reads a lot better, thanks. We could arguably convert the
if-else-chain into a switch to make all of this read a bit better, but
that is a subjective style change and definitely not something that you
have to do as part of this series.
I concur. I admit that using switch never occured to me but I agree
100% with you that it would make the result nicer, and that it does
not have to be part of this series.
One thing I wondered is whether it's okay to not die anymore via
`BUG()`. The other error cases already don't die though, so this ought
to be fine. Going up the callchain shows that we do bubble up the error
as expected until we end up in `match_push_refs()`. There's multiple
callers of that function, and all except one perform error handling for
it.
The only exception is git-remote(1) in `get_push_ref_states()`, where it
gets executed via `git remote show $remote_name`. As far as I understand
we would end up not showing any references that are broken, and we would
print the above advise. Which I think is reasonable.
So all of this looks good to me, thanks!
Nice to see somebody thinks through the potential impact for all the
callers. Very much appreciated.
Let's merge the topic to 'next'.
Thanks.