doc: remove confusing command to send patch

Message ID 20231010162635.755975-1-thomas@monjalon.net (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series doc: remove confusing command to send patch |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Thomas Monjalon Oct. 10, 2023, 4:26 p.m. UTC
  In the contributor guide, it was said that no need to Cc maintainers
for new additions, probably for new directories not having a maintainer.
There is no harm, and it is a good habit, to always Cc maintainers.

Remove this case as it can mislead to not Cc maintainers when needed.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 doc/guides/contributing/patches.rst | 4 ----
 1 file changed, 4 deletions(-)
  

Comments

fengchengwen Oct. 11, 2023, 2:04 a.m. UTC | #1
Acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 2023/10/11 0:26, Thomas Monjalon wrote:
> In the contributor guide, it was said that no need to Cc maintainers
> for new additions, probably for new directories not having a maintainer.
> There is no harm, and it is a good habit, to always Cc maintainers.
> 
> Remove this case as it can mislead to not Cc maintainers when needed.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  doc/guides/contributing/patches.rst | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
> index e286d9e6d5..d114ceba2b 100644
> --- a/doc/guides/contributing/patches.rst
> +++ b/doc/guides/contributing/patches.rst
> @@ -561,10 +561,6 @@ Script ``get-maintainer.sh`` can be used to select maintainers automatically::
>  
>    git send-email --to-cmd ./devtools/get-maintainer.sh --cc dev@dpdk.org 000*.patch
>  
> -New additions can be sent without a maintainer::
> -
> -   git send-email --to dev@dpdk.org 000*.patch
> -
>  You can test the emails by sending it to yourself or with the ``--dry-run`` option.
>  
>  If the patch is in relation to a previous email thread you can add it to the same thread using the Message ID::
>
  
David Marchand Oct. 11, 2023, 7:30 a.m. UTC | #2
On Tue, Oct 10, 2023 at 6:26 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> In the contributor guide, it was said that no need to Cc maintainers
> for new additions, probably for new directories not having a maintainer.
> There is no harm, and it is a good habit, to always Cc maintainers.
>
> Remove this case as it can mislead to not Cc maintainers when needed.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

I agree Cc: maintainers should be the default / recommended way of
sending patches.

Just to convince myself, adding some meson skeleton for a "plop"
library, adding an entry in the release notes and hooking in
lib/meson.build:
$ git show --stat
 doc/guides/rel_notes/release_23_11.rst | 4 ++++
 lib/meson.build                        | 1 +
 lib/plop/meson.build                   | 2 ++

$ ./devtools/get-maintainer.sh 0001-new-awesome-library.patch

In this case, it translates to an empty To: list if you follow the
example command line:
   git send-email --to-cmd ./devtools/get-maintainer.sh --cc
dev@dpdk.org 000*.patch

We could add a default list of recipients if no maintainer is found by
the script.
And the next question is who should be in that list..
  
Thomas Monjalon Oct. 11, 2023, 8:03 a.m. UTC | #3
11/10/2023 09:30, David Marchand:
> On Tue, Oct 10, 2023 at 6:26 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > In the contributor guide, it was said that no need to Cc maintainers
> > for new additions, probably for new directories not having a maintainer.
> > There is no harm, and it is a good habit, to always Cc maintainers.
> >
> > Remove this case as it can mislead to not Cc maintainers when needed.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> I agree Cc: maintainers should be the default / recommended way of
> sending patches.
> 
> Just to convince myself, adding some meson skeleton for a "plop"
> library, adding an entry in the release notes and hooking in
> lib/meson.build:
> $ git show --stat
>  doc/guides/rel_notes/release_23_11.rst | 4 ++++
>  lib/meson.build                        | 1 +
>  lib/plop/meson.build                   | 2 ++
> 
> $ ./devtools/get-maintainer.sh 0001-new-awesome-library.patch
> 
> In this case, it translates to an empty To: list if you follow the
> example command line:
>    git send-email --to-cmd ./devtools/get-maintainer.sh --cc
> dev@dpdk.org 000*.patch
> 
> We could add a default list of recipients if no maintainer is found by
> the script.
> And the next question is who should be in that list..

Or we can send to dev@dpdk.org, Cc maintainers.
This is what I do:
git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh
  
Bruce Richardson Oct. 11, 2023, 8:30 a.m. UTC | #4
On Wed, Oct 11, 2023 at 10:03:07AM +0200, Thomas Monjalon wrote:
> 11/10/2023 09:30, David Marchand:
> > On Tue, Oct 10, 2023 at 6:26 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > In the contributor guide, it was said that no need to Cc maintainers
> > > for new additions, probably for new directories not having a maintainer.
> > > There is no harm, and it is a good habit, to always Cc maintainers.
> > >
> > > Remove this case as it can mislead to not Cc maintainers when needed.
> > >
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > 
> > I agree Cc: maintainers should be the default / recommended way of
> > sending patches.
> > 
> > Just to convince myself, adding some meson skeleton for a "plop"
> > library, adding an entry in the release notes and hooking in
> > lib/meson.build:
> > $ git show --stat
> >  doc/guides/rel_notes/release_23_11.rst | 4 ++++
> >  lib/meson.build                        | 1 +
> >  lib/plop/meson.build                   | 2 ++
> > 
> > $ ./devtools/get-maintainer.sh 0001-new-awesome-library.patch
> > 
> > In this case, it translates to an empty To: list if you follow the
> > example command line:
> >    git send-email --to-cmd ./devtools/get-maintainer.sh --cc
> > dev@dpdk.org 000*.patch
> > 
> > We could add a default list of recipients if no maintainer is found by
> > the script.
> > And the next question is who should be in that list..
> 
> Or we can send to dev@dpdk.org, Cc maintainers.
> This is what I do:
> git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh
> 
+1 for this, mainly on the basis of it being what I do too! :-)
  
Ferruh Yigit Oct. 11, 2023, 8:41 a.m. UTC | #5
On 10/11/2023 9:30 AM, Bruce Richardson wrote:
> On Wed, Oct 11, 2023 at 10:03:07AM +0200, Thomas Monjalon wrote:
>> 11/10/2023 09:30, David Marchand:
>>> On Tue, Oct 10, 2023 at 6:26 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>>>
>>>> In the contributor guide, it was said that no need to Cc maintainers
>>>> for new additions, probably for new directories not having a maintainer.
>>>> There is no harm, and it is a good habit, to always Cc maintainers.
>>>>
>>>> Remove this case as it can mislead to not Cc maintainers when needed.
>>>>
>>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>>>
>>> I agree Cc: maintainers should be the default / recommended way of
>>> sending patches.
>>>
>>> Just to convince myself, adding some meson skeleton for a "plop"
>>> library, adding an entry in the release notes and hooking in
>>> lib/meson.build:
>>> $ git show --stat
>>>  doc/guides/rel_notes/release_23_11.rst | 4 ++++
>>>  lib/meson.build                        | 1 +
>>>  lib/plop/meson.build                   | 2 ++
>>>
>>> $ ./devtools/get-maintainer.sh 0001-new-awesome-library.patch
>>>
>>> In this case, it translates to an empty To: list if you follow the
>>> example command line:
>>>    git send-email --to-cmd ./devtools/get-maintainer.sh --cc
>>> dev@dpdk.org 000*.patch
>>>
>>> We could add a default list of recipients if no maintainer is found by
>>> the script.
>>> And the next question is who should be in that list..
>>
>> Or we can send to dev@dpdk.org, Cc maintainers.
>> This is what I do:
>> git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh
>>
> +1 for this, mainly on the basis of it being what I do too! :-)
>

I am for "--to-cmd=./devtools/get-maintainer.sh --cc dev@dpdk.org"

To highlight response is expected from the maintainers, and community is
informed.

Also people may have filters to give higher priority to emails they are
in 'to' list, high priority is what we want from maintainers :)
  
Thomas Monjalon Oct. 11, 2023, 10:20 a.m. UTC | #6
11/10/2023 10:41, Ferruh Yigit:
> On 10/11/2023 9:30 AM, Bruce Richardson wrote:
> > On Wed, Oct 11, 2023 at 10:03:07AM +0200, Thomas Monjalon wrote:
> >> 11/10/2023 09:30, David Marchand:
> >>> On Tue, Oct 10, 2023 at 6:26 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >>>>
> >>>> In the contributor guide, it was said that no need to Cc maintainers
> >>>> for new additions, probably for new directories not having a maintainer.
> >>>> There is no harm, and it is a good habit, to always Cc maintainers.
> >>>>
> >>>> Remove this case as it can mislead to not Cc maintainers when needed.
> >>>>
> >>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> >>>
> >>> I agree Cc: maintainers should be the default / recommended way of
> >>> sending patches.
> >>>
> >>> Just to convince myself, adding some meson skeleton for a "plop"
> >>> library, adding an entry in the release notes and hooking in
> >>> lib/meson.build:
> >>> $ git show --stat
> >>>  doc/guides/rel_notes/release_23_11.rst | 4 ++++
> >>>  lib/meson.build                        | 1 +
> >>>  lib/plop/meson.build                   | 2 ++
> >>>
> >>> $ ./devtools/get-maintainer.sh 0001-new-awesome-library.patch
> >>>
> >>> In this case, it translates to an empty To: list if you follow the
> >>> example command line:
> >>>    git send-email --to-cmd ./devtools/get-maintainer.sh --cc
> >>> dev@dpdk.org 000*.patch
> >>>
> >>> We could add a default list of recipients if no maintainer is found by
> >>> the script.
> >>> And the next question is who should be in that list..
> >>
> >> Or we can send to dev@dpdk.org, Cc maintainers.
> >> This is what I do:
> >> git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh
> >>
> > +1 for this, mainly on the basis of it being what I do too! :-)
> >
> 
> I am for "--to-cmd=./devtools/get-maintainer.sh --cc dev@dpdk.org"
> 
> To highlight response is expected from the maintainers, and community is
> informed.
> 
> Also people may have filters to give higher priority to emails they are
> in 'to' list, high priority is what we want from maintainers :)

They should give high priority when they are Cc as well.

The problem is that we may have patches with empty "To",
especially for cover letters and new libs.
  
Ferruh Yigit Oct. 11, 2023, 10:22 a.m. UTC | #7
On 10/11/2023 11:20 AM, Thomas Monjalon wrote:
> 11/10/2023 10:41, Ferruh Yigit:
>> On 10/11/2023 9:30 AM, Bruce Richardson wrote:
>>> On Wed, Oct 11, 2023 at 10:03:07AM +0200, Thomas Monjalon wrote:
>>>> 11/10/2023 09:30, David Marchand:
>>>>> On Tue, Oct 10, 2023 at 6:26 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>>>>>
>>>>>> In the contributor guide, it was said that no need to Cc maintainers
>>>>>> for new additions, probably for new directories not having a maintainer.
>>>>>> There is no harm, and it is a good habit, to always Cc maintainers.
>>>>>>
>>>>>> Remove this case as it can mislead to not Cc maintainers when needed.
>>>>>>
>>>>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>>>>>
>>>>> I agree Cc: maintainers should be the default / recommended way of
>>>>> sending patches.
>>>>>
>>>>> Just to convince myself, adding some meson skeleton for a "plop"
>>>>> library, adding an entry in the release notes and hooking in
>>>>> lib/meson.build:
>>>>> $ git show --stat
>>>>>  doc/guides/rel_notes/release_23_11.rst | 4 ++++
>>>>>  lib/meson.build                        | 1 +
>>>>>  lib/plop/meson.build                   | 2 ++
>>>>>
>>>>> $ ./devtools/get-maintainer.sh 0001-new-awesome-library.patch
>>>>>
>>>>> In this case, it translates to an empty To: list if you follow the
>>>>> example command line:
>>>>>    git send-email --to-cmd ./devtools/get-maintainer.sh --cc
>>>>> dev@dpdk.org 000*.patch
>>>>>
>>>>> We could add a default list of recipients if no maintainer is found by
>>>>> the script.
>>>>> And the next question is who should be in that list..
>>>>
>>>> Or we can send to dev@dpdk.org, Cc maintainers.
>>>> This is what I do:
>>>> git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh
>>>>
>>> +1 for this, mainly on the basis of it being what I do too! :-)
>>>
>>
>> I am for "--to-cmd=./devtools/get-maintainer.sh --cc dev@dpdk.org"
>>
>> To highlight response is expected from the maintainers, and community is
>> informed.
>>
>> Also people may have filters to give higher priority to emails they are
>> in 'to' list, high priority is what we want from maintainers :)
> 
> They should give high priority when they are Cc as well.
> 
> The problem is that we may have patches with empty "To",
> especially for cover letters and new libs.
> 

There are indeed, for those cases I am putting 'dev' to "To:".
  
Stephen Hemminger Oct. 11, 2023, 3:52 p.m. UTC | #8
On Wed, 11 Oct 2023 09:30:23 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> > Or we can send to dev@dpdk.org, Cc maintainers.
> > This is what I do:
> > git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh
> >   
> +1 for this, mainly on the basis of it being what I do too! :-)

Ditto.
Maybe would be good to have alias or wrapper script?
  
Thomas Monjalon Oct. 11, 2023, 4:02 p.m. UTC | #9
11/10/2023 17:52, Stephen Hemminger:
> On Wed, 11 Oct 2023 09:30:23 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > > Or we can send to dev@dpdk.org, Cc maintainers.
> > > This is what I do:
> > > git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh
> > >   
> > +1 for this, mainly on the basis of it being what I do too! :-)
> 
> Ditto.
> Maybe would be good to have alias or wrapper script?

I am convinced we should have an interactive script to send patches.
I could make something using this tool:
	https://github.com/tmonjalo/choose
  
Thomas Monjalon Oct. 11, 2023, 4:03 p.m. UTC | #10
11/10/2023 12:22, Ferruh Yigit:
> On 10/11/2023 11:20 AM, Thomas Monjalon wrote:
> > 11/10/2023 10:41, Ferruh Yigit:
> >> On 10/11/2023 9:30 AM, Bruce Richardson wrote:
> >>> On Wed, Oct 11, 2023 at 10:03:07AM +0200, Thomas Monjalon wrote:
> >>>> 11/10/2023 09:30, David Marchand:
> >>>>> On Tue, Oct 10, 2023 at 6:26 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >>>>>>
> >>>>>> In the contributor guide, it was said that no need to Cc maintainers
> >>>>>> for new additions, probably for new directories not having a maintainer.
> >>>>>> There is no harm, and it is a good habit, to always Cc maintainers.
> >>>>>>
> >>>>>> Remove this case as it can mislead to not Cc maintainers when needed.
> >>>>>>
> >>>>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> >>>>>
> >>>>> I agree Cc: maintainers should be the default / recommended way of
> >>>>> sending patches.
> >>>>>
> >>>>> Just to convince myself, adding some meson skeleton for a "plop"
> >>>>> library, adding an entry in the release notes and hooking in
> >>>>> lib/meson.build:
> >>>>> $ git show --stat
> >>>>>  doc/guides/rel_notes/release_23_11.rst | 4 ++++
> >>>>>  lib/meson.build                        | 1 +
> >>>>>  lib/plop/meson.build                   | 2 ++
> >>>>>
> >>>>> $ ./devtools/get-maintainer.sh 0001-new-awesome-library.patch
> >>>>>
> >>>>> In this case, it translates to an empty To: list if you follow the
> >>>>> example command line:
> >>>>>    git send-email --to-cmd ./devtools/get-maintainer.sh --cc
> >>>>> dev@dpdk.org 000*.patch
> >>>>>
> >>>>> We could add a default list of recipients if no maintainer is found by
> >>>>> the script.
> >>>>> And the next question is who should be in that list..
> >>>>
> >>>> Or we can send to dev@dpdk.org, Cc maintainers.
> >>>> This is what I do:
> >>>> git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh
> >>>>
> >>> +1 for this, mainly on the basis of it being what I do too! :-)
> >>>
> >>
> >> I am for "--to-cmd=./devtools/get-maintainer.sh --cc dev@dpdk.org"
> >>
> >> To highlight response is expected from the maintainers, and community is
> >> informed.
> >>
> >> Also people may have filters to give higher priority to emails they are
> >> in 'to' list, high priority is what we want from maintainers :)
> > 
> > They should give high priority when they are Cc as well.
> > 
> > The problem is that we may have patches with empty "To",
> > especially for cover letters and new libs.
> 
> There are indeed, for those cases I am putting 'dev' to "To:".

So it would be simpler to recommend a single method with maintainers as Cc.
  

Patch

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index e286d9e6d5..d114ceba2b 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -561,10 +561,6 @@  Script ``get-maintainer.sh`` can be used to select maintainers automatically::
 
   git send-email --to-cmd ./devtools/get-maintainer.sh --cc dev@dpdk.org 000*.patch
 
-New additions can be sent without a maintainer::
-
-   git send-email --to dev@dpdk.org 000*.patch
-
 You can test the emails by sending it to yourself or with the ``--dry-run`` option.
 
 If the patch is in relation to a previous email thread you can add it to the same thread using the Message ID::