[PATH] branch.c:install_branch_config():Simplify code generating verbose message.

From: =?UTF-8?q?Pawe=C5=82=20Wawruch?= <pawlo@aleg.pl>
Date: Thu, 6 Mar 2014 00:05:00 +0100
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.

---
 branch.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

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 <pawlo@aleg.pl> wrote:

Drop these lines. They are extracted automatically by "git am" from the headers of your mail message.

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.

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.

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.

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_("..."),
        ...
    };

Move the declarations of 'name' and 'message_number' to the top of the block.

I don't know how other people feel about it, but it disturbs me a bit to see message_number computed from these other variables without ensuring that their values really are 0 or 1. I checked the implementation of the functions from which these variables are set, so I know that they are indeed 0 or 1, but doing so was an extra burden which wouldn't have been required if this code was more explicit about their values. For instance:

    int message_number = !!rebasing + 2 * !!origin + 4 * !!remote_is_branch;

Again, it disturbs me to see messages[] indexed without ensuring that message_number is within range. At the very least, an

    assert(message_number < ARRAY_SIZE(messages));

after computing message_number's value would help to protect against programmer error in future changes.