Re: [PATCH 1/2] Documentation/SubmittingPatches: Explain the rationale of git notes

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

 



On Mon, Dec 22, 2014 at 9:55 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> This adds an explanation of why you want to have the --notes option
>> given to git format-patch.
>>
>> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
>> ---
>>
>> Notes:
>>     > with optionally update Documentation/SubmittingPatches
>>     > to point at the file.
>>
>>     That file actually talks about notes already.  I am sending
>>     two patches touching that file, one of them explaining
>>     the --notes workflow rationale and one of them just changing
>>     white spaces.
>>
>>     A few weeks ago I wanted to patch format-patch to remove
>>     change ids. This is not needed any more for the git workflow
>>     as I disabled them and do not upload any patches to an optional
>>     Gerrit code review server anymore.
>>
>>     I do like the workflow using --notes as well from a developers
>>     perspective as I take literally notes for my own sanity.
>>     I wonder if I should add a config format.notes = [default-off,
>>     always, on-if-non-empty] so I don't need always add --notes
>>     manually to the command line.
>>
>>     Thanks,
>>     Stefan
>>
>>  Documentation/SubmittingPatches | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
>> index fa71b5f..16b5d65 100644
>> --- a/Documentation/SubmittingPatches
>> +++ b/Documentation/SubmittingPatches
>> @@ -176,7 +176,11 @@ message starts, you can put a "From: " line to name that person.
>>  You often want to add additional explanation about the patch,
>>  other than the commit message itself.  Place such "cover letter"
>>  material between the three dash lines and the diffstat. Git-notes
>> -can also be inserted using the `--notes` option.
>> +can also be inserted using the `--notes` option. If you are one
>
> Because the previous sentence is talking about the cover letter to
> describe the overall series, it probably is a good idea to add
> "after the three-dashes of each patch" after "... using the notes
> option" for clarity, perhaps?

The previous sentence is talking about additional information,
which should not go into the commit message. (very similar to
the cover letter but just for one patch). It already mentions the place
"between the three dash lines and the diffstat."

>
>> +of those developers who cannot write perfect code the first time
>> +and need multiple iterations of review and discussion, you are
>> +encouraged to use the notes to describe the changes between the
>> +different versions of a patch.
>
> Perhaps s/you are encouraged to/you may want to/?  It might be
> better to be even more explicit, e.g. "you may want to keep track of
> the changes between the versions using the notes and then use
> `--notes` when preparing the series for submission"?

I did reword it to make it more obvious.

>
>>  Do not attach the patch as a MIME attachment, compressed or not.
>>  Do not let your e-mail client send quoted-printable.  Do not let
>
> Thanks.

A paragraph before that change we have

    "git format-patch" command follows the best current practice to
    format the body of an e-mail message.

which makes me wonder if the notes are a best practice or may be
becoming a best practice. So maybe we want to default to add the notes
in format-patch instead of explicitly asking for it. But that's just an idea
suited for another patch.

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