From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 26 Mar 2009 06:49:19 GMT
Subject: Re: [PATCH 08/10] bisect--helper: implement "git bisect--helper"
Message-ID: <7v3ad0srrk.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20090326055554.e91bf6ba.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> This patch implements a new "git bisect--helper" builtin plumbing
> command that will be used to migrate "git-bisect.sh" to C.
>
> We start by implementing only the "--next-vars" option that will
> read bisect refs from "refs/bisect/", and then compute the next
> bisect step, and output shell variables ready to be eval'ed by
> the shell.
>
> At this step, "git bisect--helper" ignores the paths that may
> have been put in "$GIT_DIR/BISECT_NAMES". This will be fixed in a
> later patch.

Very nicely done.

> +static int read_bisect_refs(void)
> +{
> +	return for_each_bisect_ref(register_ref, NULL);
> +}

This is only a minor point, but I do not foresee anybody other than
bisect--helper (and later bisect) running for_each_bisect_ref().  It might
make sense to redo [01/10] to introduce

	for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb)

and change this call site to:

	return for_each_ref_in("refs/bisect/", register_ref, NULL);

Needless to say, for_each_{ref,tag_ref,branch_ref,remote_ref}() can be
redefined in terms of for_each_ref_in() so that we can lose these
hardcoded length of prefix strings from the code.