Re: [PATCH 0/5] Start of a journey: drop NO_THE_INDEX_COMPATIBILITY_MACROS

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Stefan & Junio,

On Thu, 4 May 2017, Stefan Beller wrote:

> So instead of a mechanical replacement, we'd rather want to
> see "the_index" not appearing at all outside of builtins, which
> implies two things:
>
> * If done properly we can move the macros from cache.h to
>   e.g. builtin.h. That way future developers are less tempted
>   to use the cache_* macros in the library code.

Yessss!

> * we'd have to pass through the_index from the builtin function
>   down to the library code, potentially going through multiple
>   function. For this it is unclear if we want to start this now, or wait
>   until Brandon presents his initial repository object struct, which
>   may be suited better for passing-around.

Or the other way round. I guess passing a struct index_state can be a
first step, and we can later convert it to struct repository. I fathom
that more places will need a struct repository parameter than a struct
index_state parameter. That is, if you first identify all the places where
the index_state parameter is required, it should make the struct
repository change easier.

> So if I want to further look into refactoring, I'll have a look into
> the object store and hold off sending a series that drops the_index.
> 
> > Also, dropping compatibility macros at the end of the series is
> > unfriendly to fellow developers with WIPs that depend on the
> > traditional way of doing things.  It is doubly insulting to them to
> > send such a series during the feature freeze period, when it is more
> > likely that they are holding off their WIP in an attempt to allow us
> > to concentrate more on what we should be, i.e. finding and fixing
> > possible regressions at the tip of 'master' to polish the upcoming
> > release and help the end users.
> 
> Personally I have not noticed large variations of patch volume
> correlated to pre-release times.

Speaking for myself, I also use this "slow" time to prepare patch series
that should be submitted directly after a major version bump, patch series
like the timestamp_t one (with which I failed miserably: it got to a
usable state only now, very short *before* a major version bump).

But yes, part of why I set aside a substantial chunk of time to work on
the Coverity report was to prepare for the major version, to make sure
that we did not have anything blatant (like the difftool use-after-free,
which was embarrassing) in v2.13.0.

It may look as if the coverity patches do not focus on critical fixes, but
that's only because I did not find anything major in the Coverity report
(I looked primarily for Git for Windows-specific stuff, though, to be
honest).

Maybe it is a similar situation for other patch contributors: they tried
to focus on critical fixes and ended up not finding anything really
critical?

Having said that, I did see a little shift toward cppcheck & sparse
related patches ;-)

Ciao,
Dscho