Re: [PATCHv2] unpack-trees: move checkout state into check_updates
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 1, 2017, 01:26 UTC
- Message-ID
- <xmqq8tqv6051.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20161229194309.2373-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
Show 12 quoted lines
> The checkout state was introduced via 16da134b1f9 > (read-trees: refactor the unpack_trees() part, 2006-07-30). An attempt to > refactor the checkout state was done in b56aa5b268e (unpack-trees: pass > checkout state explicitly to check_updates(), 2016-09-13), but we can > go even further. > > The `struct checkout state` is not used in unpack_trees apart from > initializing it, so move it into the function that makes use of it, > which is `check_updates`. > > Signed-off-by: Stefan Beller <sbeller@google.com> > ---
I'd add René's Reviewed-by: here.
Show 16 quoted lines
> -static int check_updates(struct unpack_trees_options *o,
> - const struct checkout *state)
> +static int check_updates(struct unpack_trees_options *o)
> {
> unsigned cnt = 0, total = 0;
> struct progress *progress = NULL;
> struct index_state *index = &o->result;
> - int i;
> - int errs = 0;
> + struct checkout state = CHECKOUT_INIT;
> + int i, errs = 0;
> +
> + state.force = 1;
> + state.quiet = 1;
> + state.refresh_cache = 1;
> + state.istate = index;I think moving heavier and initialized variables earlier and more lightweight and ephemeral ones like "i" later does make it easier to follow. "errs" has the significance and the lifetime similar to cnt/total, and logically should be higher, though. It is not a big enough deal to reroll (but as your futzing of the variable definition order was not a big enough deal to do in this patch, either, so...).
Queued. Thanks.