Re: [PATCH v2 2/4] Add format.coverauto boolean
- From
- Frank Terbeck <ft@bewatermyfriend.org>
- Date
- May 4, 2009, 21:41 UTC
- Message-ID
- <20090504214149.GA26208@fsst.voodoo.lan>
- In-Reply-To
- <780e0a6b0905041139x3a764768ocbbd8d9353ea568f@mail.gmail.com>
Stephen Boyd <bebarino@gmail.com>:
Show 10 quoted lines
> On Mon, May 4, 2009 at 2:59 AM, Frank Terbeck <ft@bewatermyfriend.org> wrote: > > An exception is if it is called using the --stdout option, > > which disables format.coverauto, because users of --stdout > > (like git-rebase.sh) usually are not interested in > > cover letters at all. > > > > Would it make more sense to just have git-rebase.sh use > --cover-letter=never? I thought configuration variables were defaults > which have to be overridden.
Could be done. And in an earlier version I did that. Since that took changes in more places, I reverted to this less intrusive approach.
Also, this will keep the scripts of people who use --stdout working, no matter what the settings of an individual user might be.
I don't think people who use --stdout will want the cover letter (which is always the same) in the output. Since you can still force its output, I think this would be reasonable compromise.
> Also, why does this variable even exist? I think Jeff's suggestion is > best, where you can set format.coverletter to always, never, or some > number.
Well, I did this because I wanted both Junio's and Jeff's suggestions to be incorporated.
Junio correctly stated, that nobody will usually want cover letters for one-patch "series". Which I think is right; and the wrapper I used for this before did take that into account as well. So coverletter had to default to 2.
If that where to enable the generation of cover letters for all format-patch calls with every patch series that is at least two patches long, it would change format-patch's default behaviour; potentially breaking people's scripts (similarly to the way coverauto=true could break git-rebase.sh without either adding --cover-letter=never or the exception in the --stdout codepath).
That's why I think this should be handled in two separate options. And as I mentioned, coverauto could be left out if we'd advise users who want that to create an git alias that does 'format-patch --cover-letter'.
Show 6 quoted lines
> > + if (!strcmp(var, "format.coverauto")) {
> > + cover_letter = git_config_bool(var, value);;
> > + return 0;
> > + }
>
> Double semi-colon?Oops, yeah. I'll resend this one (if coverauto turns out to be the way to go).
Regards, Frank
--
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
-- RFC 1925