On Mon, Dec 03, 2018 at 11:23:56AM -0800, Matthew DeVore wrote:
> 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 <[email protected]>
> ---
> builtin/pack-objects.c | 7 +++++--
> builtin/prune.c | 1 -
> builtin/rev-list.c | 6 ++++--
> revision.c | 10 ++++++----
> revision.h | 4 ++--
> 5 files changed, 17 insertions(+), 11 deletions(-)
Thanks, this mostly looks good to me (with or without tweaking the
initializers as discussed in the other part of the thread).
One thing I noticed:
> @@ -297,7 +296,8 @@ struct setup_revision_opt {
> const char *def;
> void (*tweak)(struct rev_info *, struct setup_revision_opt *);
> const char *submodule; /* TODO: drop this and use rev_info->repo */
> - int assume_dashdash;
> + int assume_dashdash : 1;
> + int allow_exclude_promisor_objects : 1;
> unsigned revarg_opt;
> };
I don't know that we need to penny-pinch bytes in this struct, but in
general it shouldn't hurt either awy. However, a signed bit-field with 1
bit is funny. I'm not even sure what the standard has to say, but in
twos-complement that would store "-1" and "0" (gcc -Wpedantic also
complains about overflow in assigning "1" to it).
So this probably ought to be "unsigned".
-Peff
