From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 14 Jul 2016 15:45:13 GMT
Subject: Re: What's cooking in git.git (Jul 2016, #05; Wed, 13)
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.

> +               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.