[dpdk-dev] [PATCH] doc: update git fixline alias with cc to stable

[dpdk-dev] [PATCH] doc: update git fixline alias with cc to stable

[Reshma Pattan]

Update git fixline alias to add stable@dpdk.org to Cc

Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 doc/guides/contributing/patches.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index 214030346..fa19d2eb5 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -255,7 +255,7 @@ Here are some guidelines for the body of a commit message:
   You can generate the required lines using the following git alias, which prints
   the commit SHA and the author of the original code::
 
-     git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'"
+     git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae%nCc: stable@dpdk.org'"
 
   The output of ``git fixline <SHA>`` must then be added to the commit message::
 
-- 
2.21.0

Re: [dpdk-dev] [PATCH] doc: update git fixline alias with cc to stable

[Bruce Richardson]

On Tue, Nov 19, 2019 at 11:03:57AM +0000, Reshma Pattan wrote:
> Update git fixline alias to add stable@dpdk.org to Cc
> 
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
>  doc/guides/contributing/patches.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
> index 214030346..fa19d2eb5 100644
> --- a/doc/guides/contributing/patches.rst
> +++ b/doc/guides/contributing/patches.rst
> @@ -255,7 +255,7 @@ Here are some guidelines for the body of a commit message:
>    You can generate the required lines using the following git alias, which prints
>    the commit SHA and the author of the original code::
>  
> -     git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'"
> +     git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae%nCc: stable@dpdk.org'"
>  
>    The output of ``git fixline <SHA>`` must then be added to the commit message::
>  

While a good idea, we don't always want to CC stable for all fixes, as
fixes for commits in the current release obviously don't need backports.
I suggest the stable maintainers comment on whether receiving these
additional patches would be a problem for them, or if their tooling is
sufficiency advanced for them to ignore these without issue...

/Bruce

Re: [dpdk-dev] [PATCH] doc: update git fixline alias with cc to stable

[Kevin Traynor]

On 19/11/2019 11:22, Bruce Richardson wrote:
> On Tue, Nov 19, 2019 at 11:03:57AM +0000, Reshma Pattan wrote:
>> Update git fixline alias to add stable@dpdk.org to Cc
>>
>> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
>> ---
>>  doc/guides/contributing/patches.rst | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
>> index 214030346..fa19d2eb5 100644
>> --- a/doc/guides/contributing/patches.rst
>> +++ b/doc/guides/contributing/patches.rst
>> @@ -255,7 +255,7 @@ Here are some guidelines for the body of a commit message:
>>    You can generate the required lines using the following git alias, which prints
>>    the commit SHA and the author of the original code::
>>  
>> -     git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'"
>> +     git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae%nCc: stable@dpdk.org'"
>>  
>>    The output of ``git fixline <SHA>`` must then be added to the commit message::
>>  
> 
> While a good idea, we don't always want to CC stable for all fixes, as
> fixes for commits in the current release obviously don't need backports.
> I suggest the stable maintainers comment on whether receiving these
> additional patches would be a problem for them, or if their tooling is
> sufficiency advanced for them to ignore these without issue...
> 

If it were just applied to every fix, the tooling would catch the cases
where the offending commit is not in the stable tree. However, there are
times when authors don't want patches backported. For example, if it was
invasive and had no practical relevance in the older code, or it caused
an ABI change etc.

'Cc: stable@dpdk.org' is an indication that the patch is a candidate for
backporting. With a tag missing in error there can be follow up at
backport time, with a tag added in error the patch would be backported
and it could be a problem further down the line. Given that, I would
like to keep the current meaning of the tag so authors can indicate
their intentions.

thanks,
Kevin.

> /Bruce
> 

Re: [dpdk-dev] [PATCH] doc: update git fixline alias with cc to stable

[Thomas Monjalon]

19/11/2019 12:22, Bruce Richardson:
> On Tue, Nov 19, 2019 at 11:03:57AM +0000, Reshma Pattan wrote:
> > --- a/doc/guides/contributing/patches.rst
> > +++ b/doc/guides/contributing/patches.rst
> > -     git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'"
> > +     git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae%nCc: stable@dpdk.org'"
> >  
> >    The output of ``git fixline <SHA>`` must then be added to the commit message::
> 
> While a good idea, we don't always want to CC stable for all fixes, as
> fixes for commits in the current release obviously don't need backports.

I've done something which proposes some Cc, some time ago:

fixline = "!f () { git log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae' $1 ; ( git tag -l --contains $1 | head -n1 | sed 's,^v,,' | sed 's,-rc.*,,' ; for i in 1 2 ; do make showversion | cut -d'.' -f-2 ; done ) | uniq -u | head -n1 | sed 's,.*,Cc: stable@dpdk.org,' ;}; f"

> I suggest the stable maintainers comment on whether receiving these
> additional patches would be a problem for them, or if their tooling is
> sufficiency advanced for them to ignore these without issue...

Anyway, the developer must think about when backport is really needed.
It is a bad idea to document it as automatic.