Re: [PATCH] branch: handle errors on setting tracking branches

> I count 4 callers in the current code, and none of them currently looks
> at the return value. Your patch updates setup_tracking() to propagate
> the error, which is good. But that is called from exactly one place,
> create_branch(), which also ignores the outcome. :-/
>
> I don't think we want to die() when the config fails. That might be the
> right thing for "branch --set-upstream-to", but probably not for
> "checkout -b". I think we need to look at each call site and see whether
> we should be propagating the error back. With the hope that we would
> ultimately affect the exit code of the command.
>
> In the case of branch creation, we are in a two-step procedure: create
> the branch, then set up its tracking config. We _could_ rollback the
> first step when the second step fails, but that is probably not worth
> the complication.
>
>> Actually, there are quite a few places where we don't check the
>> return values of git_config_set and related functions. I didn't
>> go through them yet, but might do so if you deem this to be of
>> value.
>
> I think more error-checking is better than less. But I do think it's
> worth plumbing through the error codes in each case.
>
> It's tempting to just die() when the operation fails, but as discussed
> above, that's not always the most appropriate thing.
>
>>  branch.c | 24 ++++++++++++++++--------
>>  branch.h |  3 ++-
>>  2 files changed, 18 insertions(+), 9 deletions(-)
>
> The patch itself looks OK to me from a cursory read.