Patrick Steinhardt <ps@xxxxxx> 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.
