git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Couder <chriscool@tuxfamily.org>
To: Jon Seymour <jon.seymour@gmail.com>
Cc: git@vger.kernel.org, j6t@kdbg.org, gitster@pobox.com, jnareb@gmail.com
Subject: Re: [PATCH 2/5] bisect: introduce support for --no-checkout=<ref> option.
Date: Sat, 30 Jul 2011 15:49:36 +0200	[thread overview]
Message-ID: <201107301549.36999.chriscool@tuxfamily.org> (raw)
In-Reply-To: <1312014511-7157-3-git-send-email-jon.seymour@gmail.com>

On Saturday 30 July 2011 10:28:28 Jon Seymour wrote:
> diff --git a/bisect.c b/bisect.c
> index dd7e8ed..e6c99a0 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -24,6 +24,8 @@ struct argv_array {
> 
>  static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
>  static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
> +static const char *argv_update_ref[] = {"update-ref", "--no-deref",
>                                                           NULL, NULL, NULL};
> +static const char *cursor_ref = 0;

Please use "NULL" instead of "0" above.
And is this global really needed? I would prefer if you passed one more 
argument to some functions.

>  /* bits #0-15 in revision.h */
> 
> @@ -714,9 +716,19 @@ static int bisect_checkout(char *bisect_rev_hex)
>  	mark_expected_rev(bisect_rev_hex);
> 
>  	argv_checkout[2] = bisect_rev_hex;
> -	res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
> -	if (res)
> -		exit(res);
> +	if (!cursor_ref) {
> +	  res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
> +	  if (res) {
> +	    exit(res);
> +	  }

Style nit: braces are superfluous, please use:

if (res)
        exit(res); 

> +	} else {
> +	    argv_update_ref[2] = cursor_ref;
> +	    argv_update_ref[3] = bisect_rev_hex;
> +	    res = run_command_v_opt(argv_update_ref, RUN_GIT_CMD);
> +	    if (res) {
> +	      die("update-ref --no-deref %s failed on %s", cursor_ref,
> bisect_rev_hex);
> +	    }

Style nit: braces are superfluous.

> +	}

Thanks,
Christian.

  reply	other threads:[~2011-07-30 13:49 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-30  8:28 [PATCH 0/5] bisect: Add support for a --no-checkout option Jon Seymour
2011-07-30  8:28 ` [PATCH 1/5] bisect: add tests to document expected behaviour in presence of broken trees Jon Seymour
2011-07-30  8:28 ` [PATCH 2/5] bisect: introduce support for --no-checkout=<ref> option Jon Seymour
2011-07-30 13:49   ` Christian Couder [this message]
2011-07-30  8:28 ` [PATCH 3/5] bisect: introduce --no-checkout[=<ref>] support into porcelain Jon Seymour
2011-07-30 14:34   ` Christian Couder
2011-07-30 17:00     ` Jon Seymour
2011-07-31  0:36       ` Christian Couder
2011-08-01  0:53         ` Junio C Hamano
2011-08-01  4:35           ` Christian Couder
2011-07-30  8:28 ` [PATCH 4/5] bisect: add tests for the --no-checkout option Jon Seymour
2011-07-30  8:28 ` [PATCH 5/5] bisect: add documentation for --no-checkout[=<ref>] option Jon Seymour
2011-07-30  8:30 ` [PATCH 0/5] bisect: Add support for a --no-checkout option Jon Seymour
2011-07-30 13:48 ` Christian Couder
2011-07-30 13:58   ` Jon Seymour
2011-07-30 14:19     ` Christian Couder
2011-08-01  1:00       ` Junio C Hamano
2011-08-01  4:42         ` Christian Couder
2011-08-01  5:27         ` Jon Seymour
2011-08-01 17:33           ` Junio C Hamano
2011-08-01 22:19             ` Jon Seymour
2011-08-01 23:33               ` Junio C Hamano
2011-08-02  1:15                 ` Jon Seymour
2011-08-02  1:41                   ` Jon Seymour
2011-08-02 17:53                     ` Junio C Hamano
2011-08-02 21:27                       ` Jon Seymour

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201107301549.36999.chriscool@tuxfamily.org \
    --to=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=jnareb@gmail.com \
    --cc=jon.seymour@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).