Re: Enabling the diff "indent" heuristic by default

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 08, 2017 at 10:54:10AM -0400, Marc Branchaud wrote:

> On 2017-05-08 03:48 AM, Junio C Hamano wrote:
> > 
> > * mb/diff-default-to-indent-heuristics (2017-05-02) 4 commits
> >   (merged to 'next' on 2017-05-08 at 158f401a92)
> 
> I think there's a general open question about this, which is whether or not
> we should just drop the diff.indentHeuristic configuration setting
> altogether.
> 
> Peff made the point [0] that if we keep the setting then t4061 should be
> rewritten.
> 
> My instinct is to keep the setting, at least until the changed default has a
> bit of time to settle in.  So I'll re-send the topic with the renovated
> t4061.

My instinct matches that, too. It gives people like Ævar, with his
patch-id database, a way to keep compatibility if he chooses. If we were
designing from the ground up, I'd say the option is probably just
clutter, but the backwards compatibility issue means we should probably
keep it around more or less forever.

And since Junio wasn't on the other thread, I'll repeat what I wrote
there:

  I do feel a bit sad about breaking this case (or at the very least
  forcing you to set an option to retain cross-version compatibility).
  But my gut says that we don't want to lock ourselves into never
  changing the diff algorithm (and I'm sure we've done it inadvertently
  a few times over the years; even the recent switch to turning on
  renames would have had that impact).

> Both Peff [1] and Ævar [2] mentioned situations where enabling the heuristic
> has a small impact on them.  If/when this graduates, it's perhaps worth
> adding a backward-compatibility note that the default patch IDs are
> changing.  Maybe something like:
> 
> The diff "indent" heuristic is now enabled by default.  This changes the
> patch IDs calculated by git-patch-id and used by git-cherry, which could
> affect patch-based workflows that rely on previously-computed patch IDs.
> The heuristic can be disabled by setting diff.indentHeuristic to false.

I think a note like this is a good idea.

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]