Hi,

Nemina Amarasinghe wrote:

> Signed-off-by: Nemina Amarasinghe <nemi...@gmail.com>

The above is a place to explain why this is a good change.  For example:

        When it prints a message indicating what it has done,
        install_branch_config() treats the (!remote_is_branch && origin)
        and (!remote_is_branch && !origin) cases separately.  But they
        are the same, and it uses the same message for both.

        Simplify by just checking for !remote_is_branch.

        Noticed using the magic-identical-branch-checker tool.

        Signed-off-by: ...

(That's just a first random hypothetical guess --- of course please do
not use it but put your own rationale for the change there.)

[...]
> --- a/branch.c
> +++ b/branch.c
> @@ -87,16 +87,11 @@ void install_branch_config(int flag, const char *local, 
> const char *origin, cons
>                                 _("Branch %s set up to track local branch %s 
> by rebasing.") :
>                                 _("Branch %s set up to track local branch 
> %s."),
>                                 local, shortname);
> -             else if (!remote_is_branch && origin)
> +             else if (!remote_is_branch)
>                       printf_ln(rebasing ?
>                                 _("Branch %s set up to track remote ref %s by 
> rebasing.") :
>                                 _("Branch %s set up to track remote ref %s."),
>                                 local, remote);
> -             else if (!remote_is_branch && !origin)
> -                     printf_ln(rebasing ?
> -                               _("Branch %s set up to track local ref %s by 
> rebasing.") :
> -                               _("Branch %s set up to track local ref %s."),
> -                               local, remote);

Is this safe?  Today branch.c::create_branch checks that the argument
to e.g. --set-upstream-to is either in refs/heads/* or the image of
some remote, but what is making sure that remains true tomorrow?

So if changing this, I would be happier if the "if" condition were
still (!remote_is_branch && origin) so the impossible case could emit
a BUG.

Hope that helps,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to