[PATCH] simplifying branch.c:install_branch_config() if()
Signed-off-by: Nemina Amarasinghe nemi...@gmail.com --- branch.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/branch.c b/branch.c index 723a36b..fd93603 100644 --- 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); else die(BUG: impossible combination of %d and %p, remote_is_branch, origin); -- 1.9.0.152.g6ab4ae2 -- 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
Re: [PATCH] simplifying branch.c:install_branch_config() if()
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
Re: [PATCH] simplifying branch.c:install_branch_config() if()
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 Thanks for the coments Jonathan, Yes you are correct... I was just thought about how to simplify this chain of if() statement. Not the big picture. Now I understand when I change or implenet something I have to think not only about the current matter but abot the future also. Nemina -- 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