DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] devtools: remove ugly workaround from get maintainers
@ 2021-11-01 13:35 Ferruh Yigit
  2021-11-01 13:35 ` [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in " Ferruh Yigit
  2022-01-13 11:35 ` [dpdk-dev] [PATCH 1/2] devtools: remove ugly workaround from " Thomas Monjalon
  0 siblings, 2 replies; 9+ messages in thread
From: Ferruh Yigit @ 2021-11-01 13:35 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Ferruh Yigit, dev, antonio.ninodiaz

Linux kernel 'get_maintainer.pl' script supports running out of Linux
tree since commit
31bb82c9caa9 ("get_maintainer: allow usage outside of kernel tree")

As commit is a few years old now, integrating it to DPDK and removing
ugly workaround for it.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: antonio.ninodiaz@arm.com
---
 devtools/get-maintainer.sh | 26 +-------------------------
 1 file changed, 1 insertion(+), 25 deletions(-)

diff --git a/devtools/get-maintainer.sh b/devtools/get-maintainer.sh
index 85740f5afd2a..bba4d3f68db8 100755
--- a/devtools/get-maintainer.sh
+++ b/devtools/get-maintainer.sh
@@ -7,7 +7,7 @@
 # - DPDK_GETMAINTAINER_PATH
 . $(dirname $(readlink -f $0))/load-devel-config
 
-options="--no-git-fallback"
+options="--no-tree --no-git-fallback"
 options="$options --no-rolestats"
 
 print_usage () {
@@ -31,28 +31,4 @@ if [ ! -f "$DPDK_GETMAINTAINER_PATH" ] ||
 	exit 1
 fi
 
-FILES="COPYING CREDITS Kbuild"
-FOLDERS="Documentation arch include fs init ipc scripts"
-
-# Kernel script checks for some files and folders to run
-workaround () {
-	for f in $FILES; do
-		if [ ! -f $f ]; then touch $f; fi
-	done
-
-	for d in $FOLDERS; do
-		if [ ! -d $d ]; then mkdir $d; fi
-	done
-}
-
-fix_workaround () {
-	for f in $FILES; do if [ -f $f ]; then rm -f $f; fi; done
-	for d in $FOLDERS; do if [ -d $d ]; then rmdir $d; fi; done
-}
-
-# clean workaround on exit
-trap fix_workaround EXIT
-
-workaround
 $DPDK_GETMAINTAINER_PATH $options $@
-# fix_workaround called on exit by trap
-- 
2.31.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in get maintainers
  2021-11-01 13:35 [dpdk-dev] [PATCH 1/2] devtools: remove ugly workaround from get maintainers Ferruh Yigit
@ 2021-11-01 13:35 ` Ferruh Yigit
  2021-11-01 22:20   ` Thomas Monjalon
  2021-11-01 23:19   ` Stephen Hemminger
  2022-01-13 11:35 ` [dpdk-dev] [PATCH 1/2] devtools: remove ugly workaround from " Thomas Monjalon
  1 sibling, 2 replies; 9+ messages in thread
From: Ferruh Yigit @ 2021-11-01 13:35 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Ferruh Yigit, dev

'get_maintainer.pl' by default returns authors that has fixes in
relevant code, to reduce the output only maintainers from MAINTAINERS
file, disabling fixes authors, by making '--no-fixes' default.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 devtools/get-maintainer.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/devtools/get-maintainer.sh b/devtools/get-maintainer.sh
index bba4d3f68db8..e546b8fdfd51 100755
--- a/devtools/get-maintainer.sh
+++ b/devtools/get-maintainer.sh
@@ -7,7 +7,7 @@
 # - DPDK_GETMAINTAINER_PATH
 . $(dirname $(readlink -f $0))/load-devel-config
 
-options="--no-tree --no-git-fallback"
+options="--no-tree --no-git-fallback --no-fixes"
 options="$options --no-rolestats"
 
 print_usage () {
-- 
2.31.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in get maintainers
  2021-11-01 13:35 ` [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in " Ferruh Yigit
@ 2021-11-01 22:20   ` Thomas Monjalon
  2021-11-02 10:04     ` Ferruh Yigit
  2021-11-01 23:19   ` Stephen Hemminger
  1 sibling, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2021-11-01 22:20 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev

01/11/2021 14:35, Ferruh Yigit:
> 'get_maintainer.pl' by default returns authors that has fixes in
> relevant code, to reduce the output only maintainers from MAINTAINERS
> file, disabling fixes authors, by making '--no-fixes' default.

Do you mean it is not Cc'ing people who have contributed to the file?
Is it keeping Cc of people having contributed to the commit being fixed?




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in get maintainers
  2021-11-01 13:35 ` [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in " Ferruh Yigit
  2021-11-01 22:20   ` Thomas Monjalon
@ 2021-11-01 23:19   ` Stephen Hemminger
  2021-11-02 10:07     ` Ferruh Yigit
  1 sibling, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2021-11-01 23:19 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Thomas Monjalon, dev

On Mon,  1 Nov 2021 13:35:33 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> 'get_maintainer.pl' by default returns authors that has fixes in
> relevant code, to reduce the output only maintainers from MAINTAINERS
> file, disabling fixes authors, by making '--no-fixes' default.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Isn't it better to send to more people on patches rather than less.
There could be cases where a patch reverts a change and the original
author never gets notified after this.

Could you give an example? 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in get maintainers
  2021-11-01 22:20   ` Thomas Monjalon
@ 2021-11-02 10:04     ` Ferruh Yigit
  2021-11-02 11:28       ` Thomas Monjalon
  0 siblings, 1 reply; 9+ messages in thread
From: Ferruh Yigit @ 2021-11-02 10:04 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Joe Perches

On 11/1/2021 10:20 PM, Thomas Monjalon wrote:
> 01/11/2021 14:35, Ferruh Yigit:
>> 'get_maintainer.pl' by default returns authors that has fixes in
>> relevant code, to reduce the output only maintainers from MAINTAINERS
>> file, disabling fixes authors, by making '--no-fixes' default.
> 
> Do you mean it is not Cc'ing people who have contributed to the file?
> Is it keeping Cc of people having contributed to the commit being fixed?
> 

It is adding both, people contributed to the file and contributed to the commit
it is fixing. More details from tool is below [1].

Intention to cc'ing the people who introduced the commit you are fixing
makes sense.
But as far as I can see it adds all xxx-by names from the fix commit, not
just author, that may add some unrelated people.

Let me give a sample:
Commit A: ("ethdev: add namespace")
           It updates bunch of drivers and it has acks from various
           driver maintainers.

Commit B: ("net/txgbe: fix link macro")
           Fixing Commit A for one driver (txgbe)

If --fixes is used on 'Commit B', the author of 'Commit A' and all people
acked 'Commit A' is added. But none of the acks were related to 'Commit B'.
So as a result unrelated people cc'ed for 'Commit B'.


If the commit and fix commit are more narrow scope, this make sense.
So we may prefer to keep '--fixes' if we are OK to some noises in some cases.



[1]
 From 'get_maintainer.pl':
    --fixes => for patches, add signatures of commits with 'Fixes: <commit>' (default: 1 (on))


And form commit log that adding the feature:
2f5bd343694e ("scripts/get_maintainer.pl: add signatures from Fixes: <badcommit> lines in commit message")

     A Fixes: lines in a commit message generally indicate that a previous
     commit was inadequate for whatever reason.
     
     The signers of the previous inadequate commit should also be cc'd on
     this new commit so update get_maintainer to find the old commit and add
     the original signers.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in get maintainers
  2021-11-01 23:19   ` Stephen Hemminger
@ 2021-11-02 10:07     ` Ferruh Yigit
  0 siblings, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2021-11-02 10:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Thomas Monjalon, dev

On 11/1/2021 11:19 PM, Stephen Hemminger wrote:
> On Mon,  1 Nov 2021 13:35:33 +0000
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> 'get_maintainer.pl' by default returns authors that has fixes in
>> relevant code, to reduce the output only maintainers from MAINTAINERS
>> file, disabling fixes authors, by making '--no-fixes' default.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Isn't it better to send to more people on patches rather than less.
> There could be cases where a patch reverts a change and the original
> author never gets notified after this.
> 
> Could you give an example?
> 

I already gave a sample on response to Thomas, can you please check there,
to not fork the same discussion.

Thanks,
ferruh

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in get maintainers
  2021-11-02 10:04     ` Ferruh Yigit
@ 2021-11-02 11:28       ` Thomas Monjalon
  2021-11-02 12:08         ` Ferruh Yigit
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2021-11-02 11:28 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, Joe Perches, stephen

02/11/2021 11:04, Ferruh Yigit:
> On 11/1/2021 10:20 PM, Thomas Monjalon wrote:
> > 01/11/2021 14:35, Ferruh Yigit:
> >> 'get_maintainer.pl' by default returns authors that has fixes in
> >> relevant code, to reduce the output only maintainers from MAINTAINERS
> >> file, disabling fixes authors, by making '--no-fixes' default.
> > 
> > Do you mean it is not Cc'ing people who have contributed to the file?
> > Is it keeping Cc of people having contributed to the commit being fixed?
> > 
> 
> It is adding both, people contributed to the file and contributed to the commit
> it is fixing. More details from tool is below [1].
> 
> Intention to cc'ing the people who introduced the commit you are fixing
> makes sense.
> But as far as I can see it adds all xxx-by names from the fix commit, not
> just author, that may add some unrelated people.
> 
> Let me give a sample:
> Commit A: ("ethdev: add namespace")
>            It updates bunch of drivers and it has acks from various
>            driver maintainers.
> 
> Commit B: ("net/txgbe: fix link macro")
>            Fixing Commit A for one driver (txgbe)
> 
> If --fixes is used on 'Commit B', the author of 'Commit A' and all people
> acked 'Commit A' is added. But none of the acks were related to 'Commit B'.
> So as a result unrelated people cc'ed for 'Commit B'.
> 
> 
> If the commit and fix commit are more narrow scope, this make sense.
> So we may prefer to keep '--fixes' if we are OK to some noises in some cases.

Yes I prefer Cc'ing everybody.
In case where the people are not needed for sure, I don't use the script.


> [1]
>  From 'get_maintainer.pl':
>     --fixes => for patches, add signatures of commits with 'Fixes: <commit>' (default: 1 (on))
> 
> 
> And form commit log that adding the feature:
> 2f5bd343694e ("scripts/get_maintainer.pl: add signatures from Fixes: <badcommit> lines in commit message")
> 
>      A Fixes: lines in a commit message generally indicate that a previous
>      commit was inadequate for whatever reason.
>      
>      The signers of the previous inadequate commit should also be cc'd on
>      this new commit so update get_maintainer to find the old commit and add
>      the original signers.
> 






^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in get maintainers
  2021-11-02 11:28       ` Thomas Monjalon
@ 2021-11-02 12:08         ` Ferruh Yigit
  0 siblings, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2021-11-02 12:08 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Joe Perches, stephen

On 11/2/2021 11:28 AM, Thomas Monjalon wrote:
> 02/11/2021 11:04, Ferruh Yigit:
>> On 11/1/2021 10:20 PM, Thomas Monjalon wrote:
>>> 01/11/2021 14:35, Ferruh Yigit:
>>>> 'get_maintainer.pl' by default returns authors that has fixes in
>>>> relevant code, to reduce the output only maintainers from MAINTAINERS
>>>> file, disabling fixes authors, by making '--no-fixes' default.
>>>
>>> Do you mean it is not Cc'ing people who have contributed to the file?
>>> Is it keeping Cc of people having contributed to the commit being fixed?
>>>
>>
>> It is adding both, people contributed to the file and contributed to the commit
>> it is fixing. More details from tool is below [1].
>>
>> Intention to cc'ing the people who introduced the commit you are fixing
>> makes sense.
>> But as far as I can see it adds all xxx-by names from the fix commit, not
>> just author, that may add some unrelated people.
>>
>> Let me give a sample:
>> Commit A: ("ethdev: add namespace")
>>             It updates bunch of drivers and it has acks from various
>>             driver maintainers.
>>
>> Commit B: ("net/txgbe: fix link macro")
>>             Fixing Commit A for one driver (txgbe)
>>
>> If --fixes is used on 'Commit B', the author of 'Commit A' and all people
>> acked 'Commit A' is added. But none of the acks were related to 'Commit B'.
>> So as a result unrelated people cc'ed for 'Commit B'.
>>
>>
>> If the commit and fix commit are more narrow scope, this make sense.
>> So we may prefer to keep '--fixes' if we are OK to some noises in some cases.
> 
> Yes I prefer Cc'ing everybody.
> In case where the people are not needed for sure, I don't use the script.
> 

OK to drop this patch, but I think good to have first one (1/2).

> 
>> [1]
>>   From 'get_maintainer.pl':
>>      --fixes => for patches, add signatures of commits with 'Fixes: <commit>' (default: 1 (on))
>>
>>
>> And form commit log that adding the feature:
>> 2f5bd343694e ("scripts/get_maintainer.pl: add signatures from Fixes: <badcommit> lines in commit message")
>>
>>       A Fixes: lines in a commit message generally indicate that a previous
>>       commit was inadequate for whatever reason.
>>       
>>       The signers of the previous inadequate commit should also be cc'd on
>>       this new commit so update get_maintainer to find the old commit and add
>>       the original signers.
>>
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] devtools: remove ugly workaround from get maintainers
  2021-11-01 13:35 [dpdk-dev] [PATCH 1/2] devtools: remove ugly workaround from get maintainers Ferruh Yigit
  2021-11-01 13:35 ` [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in " Ferruh Yigit
@ 2022-01-13 11:35 ` Thomas Monjalon
  1 sibling, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2022-01-13 11:35 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, antonio.ninodiaz

01/11/2021 14:35, Ferruh Yigit:
> Linux kernel 'get_maintainer.pl' script supports running out of Linux
> tree since commit
> 31bb82c9caa9 ("get_maintainer: allow usage outside of kernel tree")
> 
> As commit is a few years old now, integrating it to DPDK and removing
> ugly workaround for it.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied without patch 2, thanks.




^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-01-13 11:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-01 13:35 [dpdk-dev] [PATCH 1/2] devtools: remove ugly workaround from get maintainers Ferruh Yigit
2021-11-01 13:35 ` [dpdk-dev] [PATCH 2/2] devtools: disable fixes authors in " Ferruh Yigit
2021-11-01 22:20   ` Thomas Monjalon
2021-11-02 10:04     ` Ferruh Yigit
2021-11-02 11:28       ` Thomas Monjalon
2021-11-02 12:08         ` Ferruh Yigit
2021-11-01 23:19   ` Stephen Hemminger
2021-11-02 10:07     ` Ferruh Yigit
2022-01-13 11:35 ` [dpdk-dev] [PATCH 1/2] devtools: remove ugly workaround from " Thomas Monjalon

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git