|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] docs: document patch rules
On 03.02.2022 11:31, Juergen Gross wrote:
> On 03.02.22 11:07, Jan Beulich wrote:
>> On 02.02.2022 12:44, Juergen Gross wrote:
>>> +## The commit message
>>> +
>>> +The commit message is free text describing *why* the patch is done and
>>> +*how* the goal of the patch is achieved. A good commit message will
>>> describe
>>> +the current situation, the desired goal, and the way this goal is being
>>> +achieved. Parts of that can be omitted in obvious cases.
>>> +
>>> +In case additional changes are done in the patch (like e.g. cleanups),
>>> those
>>> +should be mentioned.
>>> +
>>> +When referencing other patches (e.g. `patch xy introduced a bug ...`) those
>>> +patches should be referenced via their commit id (at least 12 digits) and
>>> the
>>> +patch subject:
>>> +
>>> + Commit 67d01cdb5518 ("x86: infrastructure to allow converting certain
>>> + indirect calls to direct ones") introduced a bug ...
>>
>> I think this should have a reference to the Fixes: tag, as generally it
>> makes the text less convoluted if it references such a tag rather than
>> spelling out hash and title a 2nd time.
>
> I think this depends on the use case. If the cited patch is in the
> Fixes: tag I agree. Sometimes a patch is cited for other reasons, e.g.
> when adding a fix similar to the one in the cited patch. I always like
> to have the commit id in such a case.
>
> Are you fine with me rephrasing the text to:
>
> When referencing other patches (e.g. `similar to patch xy ...`) those
> patches should be referenced via their commit id (at least 12 digits)
> and the patch subject, if the very same patch isn't referenced by the
> `Fixes:` tag, too:
Sounds good to me.
>>> +### Reviewed-by:
>>> +
>>> +A `Reviewed-by:` tag can only be given by a reviewer of the patch. With
>>> +responding to a sent patch adding the `Reviewed-by:` tag the reviewer
>>> +(which can be anybody) confirms to have looked thoroughly at the patch and
>>> +didn't find any issue (being it technical, legal or formal ones). If the
>>> +review is covering only some parts of the patch, those parts can optionally
>>> +be specified (multiple areas can be covered with multiple `Reviewed-by:`
>>> +tags).
>>
>> I'd prefer if the comma separated form was also explicitly mentioned
>> (and hence permitted) here. I'd even go as far as suggesting that this
>> should be the preferred form as long as line length constraints permit.
>
> OTOH this will make automated parsing harder.
>
> I'm open for both variants, just wanted to mention why I've chosen the
> multiline form initially.
Unless commas are expected to be part of such "identifiers", I don't think
I see how parsing would become meaningfully harder. When the email address
is wanted, parsing would strip # and everything following anyway.
Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |