Re: [PATCH v2] revisions.c: put promisor option in specialized struct

From: Jeff King <peff@peff.net>
Date: Dec 3, 2018, 22:17 UTC
Message-ID: <20181203221729.GC11883@sigill.intra.peff.net>
In-Reply-To: <20181203221019.237034-1-matvore@google.com>

On Mon, Dec 03, 2018 at 02:10:19PM -0800, Matthew DeVore wrote:

Show 13 quoted lines

> Put the allow_exclude_promisor_objects flag in setup_revision_opt. When
> it was in rev_info, it was unclear when it was used, since rev_info is
> passed to functions that don't use the flag. This resulted in
> unnecessary setting of the flag in prune.c, so fix that as well.
> 
> Signed-off-by: Matthew DeVore <matvore@google.com>
> ---
>  builtin/pack-objects.c |  6 ++++--
>  builtin/prune.c        |  1 -
>  builtin/rev-list.c     |  6 ++++--
>  revision.c             | 10 ++++++----
>  revision.h             |  4 ++--
>  5 files changed, 16 insertions(+), 11 deletions(-)

Thanks, this version looks good to me.

One style nit that I don't think is worth a re-roll, but that Junio might want to tweak while applying:

Show 11 quoted lines

> diff --git a/revision.c b/revision.c
> index 13e0519c02..f6b32e6a42 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1791,7 +1791,8 @@ static void add_message_grep(struct rev_info *revs, const char *pattern)
>  }
>  
>  static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
> -			       int *unkc, const char **unkv)
> +			       int *unkc, const char **unkv,
> +			       const struct setup_revision_opt* opt)

We keep the "*" with the variable name, not the type.

-Peff