Subject: Re: [PATCH v4 4/6] patch-id: make it stable against hunk reordering

From: Junio C Hamano <[email-masked]>
Date: Thu, 24 Apr 2014 11:13:19 -0700

"Michael S. Tsirkin" <[email-masked]> writes:

> On Wed, Apr 23, 2014 at 03:05:42PM -0700, Junio C Hamano wrote:
>
>> After comparing the patches 4-6 and the one that has been in 'next'
>> for a few weeks, I tried to like the new one, but I couldn't.
>
> I'm fine with the one in next too.
> I was under the impression that you wanted me to change
> the behaviour so I worked on this,...

What I wanted to see was to make sure this would not kick in unless
the user explicitly asks.  "patchid.stable" configuration variable
is very much welcome, and if it defaulted to false with or without
"diff.orderfile", that would have been very much welcome.  Then
nobody will be surprised.

>> But "diff.orderfile" configuration is all and only about the
>> producer, and does not help the case at all.
>
> OK, we can just drop that, that's easy.
>
>> Compared to it, the previous version forced people who do not have a
>> particular reason to choose between the variants to use the new
>> "stable" variant, which was a lot simpler solution.
>> 
>> The reason why I merged the previous one to 'next' was because I
>> wanted to see if the behaviour change is as grave as I thought, or I
>> was just being unnecessary cautious.  If there is no third-party
>> script that precomputes something and stores them under these hashes
>> that breaks with this change, I do not see any reason not to make
>> the new "stable" one the default.
>
> Ah okay, so we just wait a bit and see and if all is quiet the
> existing patch will graduate to master with time?
> Totally cool.
> I thought you wanted me to add the config option, but if everyone
> is happy as is, I don't need it personally at all.

... or if we see problems, then build a fix on top to introduce
"patchid.stable" that defaults to false and not linking the feature
with "diff.ordefile".

Adding a new feature turned-off by default is the safer thing to do.
When nothing changes, by defintion there won't be a new breakage.
That is the default stance this project has taken for a long time,
and what made us trusted by projects that manage their source files
using our product.

It however is to favour stability and "no surprise" over progress,
and it may not be an optimal thing to do if the new feature is
compellingly good.  In some cases like this one, we may need to
experiment to find out the need of the users, and introducing the
feature with a configuration that is explicitly opt-in is one way to
do so.  We may or may not see many people using that feature without
disrupting existing users that way.

Cooking in 'next' with the feature turned-on by default is another
way that is riskier, but in this particular case, the population
that is likely to be affected are power users, which would match the
audience of 'next'---those who still want stability but want to be
closer to the cutting edge.