Re: [PATCH 1/2] git-bisect.sh : create a file if the bisection is in old/new mode, named "BISECT_OLDNEWMODE", so it can easily be seen outside the program without having to read BISECT_TERMS. This will have to be changed in further versions if new terms are introduced.

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 5, 2015, 20:03 UTC
Message-ID: <CAPig+cQnrKEx_=WHw5gjA9SVtDObXWOJ3augZbhvNUuD8o19aA@mail.gmail.com>
In-Reply-To: <1433522061-14532-1-git-send-email-stuberl@ensimag.grenoble-inp.fr>

On Fri, Jun 5, 2015 at 12:34 PM, Louis Stuber <stuberl@ensimag.grenoble-inp.fr> wrote:

> git-bisect.sh : create a file if the bisection is in old/new mode,
> named "BISECT_OLDNEWMODE", so it can easily be seen outside the
> program without having to read BISECT_TERMS. This will have to be
> changed in further versions if new terms are introduced.

Documentation/SubmittingPatches contains instructions for how to write a good commit message. The first line should be a very brief high-level overview of the change, followed by a blank line, followed by one or more paragraphs justifying and explaining the change. Also, wrap the commit message to 70-72 columns.

This commit message doesn't do a very good job of explaining the problem this change is trying to solve or justifying why this solution is preferable. Justification is particularly important considering the ominous-sounding final sentence of the commit message (which itself seems to imply that this is not a very good change).

Show 12 quoted lines

> Signed-off-by: Louis Stuber <stuberl@ensimag.grenoble-inp.fr>
> Signed-off-by: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
> ---
> diff --git a/git-bisect.sh b/git-bisect.sh
> index 109bd65..d3d19cb 100644
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -183,6 +183,10 @@ bisect_start() {
>         then
>                 echo "$BISECT_BAD" >"$GIT_DIR/BISECT_TERMS" &&
>                 echo "$BISECT_GOOD" >>"$GIT_DIR/BISECT_TERMS"
> +               if test "$BISECT_BAD" = "new"

Nit: Unnecessary quotes around "new" make the code a bit more noisy,
thus slightly more difficult to read.

> +               then
> +                       echo "" > "$GIT_DIR/BISECT_OLDNEWMODE"

Style: Drop space after redirection operator.

If only the file's existence is important, but not its content, then you could phrase this more concisely without the 'echo'. Just use the redirection operator without any command in front of it: >"$somefile"

Same comments apply below.

Show 24 quoted lines

> +               fi
>         fi &&
>         echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
>         #
> @@ -416,6 +420,7 @@ bisect_clean_state() {
>         rm -f "$GIT_DIR/BISECT_NAMES" &&
>         rm -f "$GIT_DIR/BISECT_RUN" &&
>         rm -f "$GIT_DIR/BISECT_TERMS" &&
> +       rm -f "$GIT_DIR/BISECT_OLDNEWMODE" &&
>         # Cleanup head-name if it got left by an old version of git-bisect
>         rm -f "$GIT_DIR/head-name" &&
>         git update-ref -d --no-deref BISECT_HEAD &&
> @@ -544,7 +549,8 @@ check_and_set_terms () {
>                         if test ! -s "$GIT_DIR/BISECT_TERMS"
>                         then
>                                 echo "new" >"$GIT_DIR/BISECT_TERMS" &&
> -                               echo "old" >>"$GIT_DIR/BISECT_TERMS"
> +                               echo "old" >>"$GIT_DIR/BISECT_TERMS" &&
> +                               echo "" > "$GIT_DIR/BISECT_OLDNEWMODE"
>                         fi
>                         BISECT_BAD="new"
>                         BISECT_GOOD="old" ;;
> --
> 1.7.1