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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 24, 2014, 18:13 UTC
Message-ID: <xmqq7g6ewq80.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140424062938.GA30231@redhat.com>

"Michael S. Tsirkin" <mst@redhat.com> writes:

Show 8 quoted lines

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

Show 21 quoted lines

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