On Mon, Mar 10, 2014 at 1:32 AM, Adam <adam@xxxxxxxxxxxx> wrote:
> Simplify if chain in install_branch_config().
Nicely done. Whether the rewritten code is indeed simpler is probably
a matter of taste, however, the submission itself is well executed.
> Signed-off-by: Adam <Adam@xxxxxxxxxxxx>
On this project, use your full name if possible.
> ---
The GSoC microproject which begat this submission asks if it would
make sense to change the code to be table-driven. If you considered
this suggestion, this area just below the "---" line following your
sign-off would be the perfect place to explain your reasons for
rejecting it.
> branch.c | 46 +++++++++++++++++++++++-----------------------
> 1 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/branch.c b/branch.c
> index 723a36b..b2d59f1 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -77,29 +77,29 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
> strbuf_release(&key);
>
> if (flag & BRANCH_CONFIG_VERBOSE) {
> - if (remote_is_branch && origin)
> - printf_ln(rebasing ?
> - _("Branch %s set up to track remote branch %s from %s by rebasing.") :
> - _("Branch %s set up to track remote branch %s from %s."),
> - local, shortname, origin);
> - else if (remote_is_branch && !origin)
> - printf_ln(rebasing ?
> - _("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)
> - 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);
> + if (remote_is_branch) {
> + if (origin)
> + printf_ln(rebasing ?
> + _("Branch %s set up to track remote branch %s from %s by rebasing.") :
> + _("Branch %s set up to track remote branch %s from %s."),
> + local, shortname, origin);
> + else
> + printf_ln(rebasing ?
> + _("Branch %s set up to track local branch %s by rebasing.") :
> + _("Branch %s set up to track local branch %s."),
> + local, shortname);
> + } else {
> + if (origin)
> + 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
> + 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);
> + }
> }
> }
>
> --
> 1.7.1
--
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