Thanks for the submission. Some comments below to give you a taste of
the Git review process...
On Thu, Mar 6, 2014 at 2:58 AM, Paweł Wawruch pa...@aleg.pl wrote:
From adfcfa0a334378a6242347efc0d614fa193610db Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Wawruch?= pa...@aleg.pl
Date: Thu, 6 Mar 2014 00:05:00 +0100
Drop these lines. They are extracted automatically by git am from
the headers of your mail message.
Subject: [PATCH] branch.c:install_branch_config(): Simplify the long chain of
if statements. Threre was a long chain of if statements. The code can be more
clear. The chain is replaced with table of strings. New approach is more
compact.
The Subject: also is extracted automatically from the email, so it
should not be pasted here. Instead, when you compose your commit
message, write a very concise summary of the patch as the very first
line, then a blank line, then the meat of the description using as
many lines as needed. Wrap the lines to about 65-70 characters. git
format-patch automatically uses that concise first line as the
Subject: of the mail message. This is all explained in section (2)
Describe your changes well of Documentation/SubmittingPatches.
Write in imperative tone. Rather than There was a long..., say
There is a long Rather than The chain is replaced..., say
Replace the chain
You misspelled There as Threre.
Your patch sign-off is missing. It should be placed right here above
the --- line. See Documentation/SubmittingPatches.
---
The Git GSoC page on which you found this microproject suggests
mentioning that this is a GSoC submission so that reviewers don't
overlook it. Either do so by adding [GSoC] to the subject or by
mentioning it right here as patch commentary after the --- line.
branch.c | 38 --
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..ebc2172 100644
--- a/branch.c
+++ b/branch.c
@@ -77,29 +77,23 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_release(key);
Your patch is whitespace damaged, and thus not easily applied. This
probably resulted from pasting it into your email client. Using git
send-email avoids this problem.
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);
+ const char *messages[8] ;
Drop the space before the semicolon.
On this project, we avoid intermixing declarations with code, so move
the declaration of messages[] to the top of the block.
+ messages[0] = _(Branch %s set up to track remote branch %s
from %s by
rebasing.);
+ messages[1] = _(Branch %s set up to track remote branch %s
from %s.);
+ messages[2] = _(Branch %s set up to track local branch %s by
rebasing.);
+ messages[3] = _(Branch %s set up to track local branch %s.);
+ messages[4] = _(Branch %s set up to track remote ref %s by
rebasing.);
+ messages[5] = _(Branch %s set up to track remote ref %s.);
+ messages[6] = _(Branch %s set up to track local ref %s by
rebasing.);
+ messages[7] = _(Branch %s set up to track local ref %s.);
Rather than hardcoding the array size and then assigning the elements
individually, perhaps you can construct the array with an initializer.
Use N_(...) rather than _(...).
const char *messages[] = {
N_(...),
...
};
+ const char *name = remote_is_branch ? remote : shortname;
+ int message_number = rebasing + 2 * (origin != NULL) + 4 *
remote_is_branch;
Move the
