Re: [PATCH v2 2/4] Add format.coverauto boolean

From: Junio C Hamano <gitster@pobox.com>
Date: May 5, 2009, 10:41 UTC
Message-ID: <7vvdof25oe.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20090505084916.GB26208@fsst.voodoo.lan>

Frank Terbeck <ft@bewatermyfriend.org> writes:

Show 5 quoted lines

> ... That could potentially break
> people's existing scripts - either by new default behaviour or by the
> setting of format.coverletter of an individual user. That could still
> happen when using coverauto, so maybe my reasoning was flawed - given
> that Stephen raised the same question.

That part I agree with 100%. A separate configuration variable does not help here; it will not act as an "I know what I am doing" flag.

"rebase" is not a problem because both format-patch and rebase come from the same git and people do not mix and match. The only thing we need to do is to make sure that rebase is updated to explicitly decline the cover letter in the same or earlier version of git that adds this new option.

However, there are other people's scripts to worry about, and no matter what you do, you cannot avoid breaking them if you introduce a configuration variable that changes the behaviour of the command in a backward incompatible way. This could be a disruptive change that needs to happen at a major revision boundary, if we were to add it.

> If so, do you want coverletter to default to zero (which wouldn't
> change the default behaviour) or do you want it to default to two?

The default between 0 and 2 does not matter much. If people find this new feature useful (if not, then this new feature is not worth adding), then they will set it to non-zero themselves, and then get their scripts broken. In a sense, defaulting it to zero is just delaying an inevitable breakage.

If we were to do this, we should rather default it to two from day one and make sure we break people's scripts that (rightly) rely on format-patch not producing covers that the caller never asked, to make sure they are adjusted to decline cover explicitly. And such a release should come with a note in huge red letters that says "this new feature allows you not to say --cover on the command line, and it is far more important than keeping your scripts that run format-patch and process its output working. You must adjust your scripts to the new reality. Sorry for the inconvenience, and have a nice day."

I do not think I would stand behind such a release note, though.

For one thing, I do not see a huge need for this configuration variable. Why is "--cover" too cumbersome to type, when you are already willing to type "format-patch"? You can alias the whole thing away to make it even shorter. For another, we do not simply break people's scripts knowingly.

For this feature to go forward, somebody has to find a way not to break people's scripts even when the user uses it. One possibility is to find a nicer verb X and introduce "git X" command that does what "format-patch" does but with a better default (and syntax --- people get confused by the oldest form "git format-patch $commit", which does not behave like "git log $commit" simply because the syntax predates the modern "revision range" notation the "log" family supports, such as A..B).