Hi, Nemina Amarasinghe wrote: > Signed-off-by: Nemina Amarasinghe <neminaa@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html