Re: What's cooking in git.git (Jul 2016, #05; Wed, 13)

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 14, 2016, 15:45 UTC
Message-ID: <xmqqzipkrzp2.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.20.1607141505160.6426@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> +       ret = add_cache_entry(ce, options);
> +       if (refresh) {
>
> Should we really refresh, even if ret < 0?

As we stopped calling make_cache_entry() with REFRESH flag on, we can change this not to refresh if we want to, and I think we can skip refresh without compromising correctness. But I'd prefer to see that change as a separate "optimization" step---after all, the original before this change unconditionally refreshed before even calling add_cache_entry() and knowing the result of it.

Show 7 quoted lines

> +               struct cache_entry *nce;
> +
> +               nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);
>
> This line is overly long, but there is a *lot* of precedent for that in
> merge-recursive.c, unfortunately. So this is just a remark, not an
> objection.

Yes, I had the same objection to the original codebase while I was touching it.

Thanks.