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

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.

quoted

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