Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb

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

 



Larry D'Anna <larry@xxxxxxxxxxxxxx> writes:

> 1) make your branch
>
> 2) git format-patch --cover-letter
>
> 3) edit the cover letter
>
> 3) review the series, and realize you need to fix something, fix it.

Hmph, this begs a natural question: why didn't you review and realize that
in step (1)?

> 4) git format-patch --cover-letter again
>
> 5) edit the cover letter, *again*.  hopefully you didn't overwrite the
> old one.

This step I can understand and am very sympathetic to the cause, even
though I may not be convinced that the patch under discussion is the best
solution to the issue.

What argument are you giving to the "-o" option?  If your series changed
(e.g. inserted or deleted a commit in the middle, retitled, etc.), and
your output is going to the same directory, you would end up with files
with duplicate serial numbers and you would need to purge the old one
before your next invocation of send-email.  For this reason, people
quickly learn to either give a different -o location (so that they can
compare two versions), or to purge the old contents before running
format-patch.  If the latter, it would be sufficient to save the old
0000-cover before removing them, and if the former, the old cover is
already there.  You can cut and paste from there while editing the new
one.

The thing I found suboptimal in your approach is that most often the cover
letter is written to explain what the overall goal of the series is and
how each patch relates to each other to achieve that goal.  In order to
effectively do so, the overview format-patch leaves in 0000-cover template
file helps a lot (actually that is half the reason why it shows the
overview---the other is for the recipients).

Your approach forces the user to write the blurb part in a separate file
on blank sheet of paper _before_ running format-patch, iow, without the
help of that series overview, if they want to take advantage of your "I
don't want to lose what I wrote already" feature.  To put it another way,
people who use --cover-blurb would write suboptimal (or maybe useless)
blurb text exactly because they don't look at the series overview while
they write it---the option encourages a bad cover letter to be sent to
reviewers.

I am hoping we can do better than that.

It might be sufficient for format-patch to notice a 0000-cover file that
is already there, read the subject and blurb part and carry that forward,
instead of unconditionally writing "*** SUBJECT HERE ***" and stuff.  That
way, the user does not have to prepare a separate file before running
format-patch.

By scanning from the bottom of the existing 0000-cover file, skipping
diffstat part (easy to spot with regexp) and then skip backwards a block
of text whose lines are one of:

 (1) two space indented---that's one-line-per-commit;

 (2) empty line---separator; or

 (3) unindented line that ends with '(' number ')' ':'---the author.

The remainder would be the BLURB.  And you know it is much easier to find
where the Subject: is ;-)

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