Re: [Xen-devel] [PATCH v4 1/2] xen/arm: extend fdt_property_interrupts

[Lars Kurth <lars.kurth.xen@xxxxxxxxx>]

> On 1 Aug 2019, at 15:02, Julien Grall <julien.grall@xxxxxxx> wrote:
> 
> Hi Lars,
> 
> On 01/08/2019 14:59, Lars Kurth wrote:
>>> On 1 Aug 2019, at 13:41, Julien Grall <julien.grall@xxxxxxx 
>>> <mailto:julien.grall@xxxxxxx>> wrote:
>>> 
>>> Hi Viktor,
>>> 
>>> On 01/08/2019 13:26, Viktor Mitin wrote:
>>>> Hi Julien and Volodymyr,
>>>> On Wed, Jul 31, 2019 at 3:52 PM Julien Grall <julien.grall@xxxxxxx 
>>>> <mailto:julien.grall@xxxxxxx>> wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>>>> It is recommended (and probably required, but I can't find exact place
>>>>>>> in the rules) to include cover letter if you are sending more that one
>>>>>>> patch in series. This will ease up review process, because reviewer will
>>>>>>> know what to expect in the series.
>>>>>>  > There is no such requirement, only recommendation.
>>>>> 
>>>>> It is a strong recommendation: "If you need to send more than one patches 
>>>>> (which
>>>>> normally means you're sending a patch series with cover letter),".
>>>>> 
>>>>>> I did not put it since this is simple short patch series and both
>>>>>> patches in this series have been discussed previously, so it is known
>>>>>> what it is about.
>>>>> 
>>>>> For a first, if you don't have a cover letter then the threading in e-mail
>>>>> client would look weird:
>>>>>     [PATCH v4 1/2] xen/arm: extend fdt_property_interrupts
>>>>>        |-> [PATCH v4 2/2] xen/arm: merge make_timer_node and 
>>>>> make_timer_domU_node
>>>>> 
>>>>> I tend to hid anything within the thread so I have only one title. For 
>>>>> the title
>>>>> it is not clear to me what's the purpose of the e-mail.
>>>>> 
>>>>> The cover letter is also used to keep a summary of what was discussed and 
>>>>> the
>>>>> overall goal. It does not matter if it is just a few lines. This is also 
>>>>> a good
>>>>> place to have a discussion of the overall series (i.e not specific to a 
>>>>> patch).
>>>>> 
>>>>> Lastly, you may have new reviewers that haven't followed the previous
>>>>> discussion. You have also reviewers like me which receive a few hundreds 
>>>>> e-mails
>>>>> per week (just counting my inbox so e-mail I am CCed to). While I have a 
>>>>> good
>>>>> memory, I can't possibly remember everything single e-mails.
>>>>> 
>>>>> So the cover letter is a good place to explain what changes have been done
>>>>> between series. You can also do that per-patch.
>>>>> 
>>>>> Speaking about changelog, I would highly recommend to keep all the 
>>>>> changelog
>>>>> from v1. This gives us an idea what happen over the review.
>>>> Thank you for this great and detailed argumentation provided. It makes
>>>> sense, so probably Xen patches wiki should be updated with this
>>>> information and cover letter should become not a recommendation, but a
>>>> rule.
>>> 
>>> Update to the wiki are always welcomed.
>> I still have an action to rework 
>> https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches and 
>> <https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patchesand> migrate 
>> the content to the sphinx docs
>> @Victor: can you quickly point out where we recommend to use cover letters 
>> (if you remember). I thought it was a requirement
> 
> It is not explicitly written in the wiki page. But it is implied in a few 
> places such as in the section "Providing a git branch", "Using git send-email 
> alone".

You are right. That should be made explicit. I think you are the only person in 
years that sent a series without cover letter

I will have a go over that page in the next few days reducing the options and 
making it more strict and clear

It would be good if you could go over it, once I am done, and let me know 
whether it is clearer

Lars



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel