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

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

 



Frank Terbeck <ft@xxxxxxxxxxxxxxxxxxx> writes:

> ... 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).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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]