[Xen-devel,for-4.11] scripts/add_maintainers.pl: Don't call get_maintainers.pl with -f

Message ID 20180605163938.28090-1-julien.grall@arm.com
State New
Headers show
Series
  • [Xen-devel,for-4.11] scripts/add_maintainers.pl: Don't call get_maintainers.pl with -f
Related show

Commit Message

Julien Grall June 5, 2018, 4:39 p.m.
The option -f of scripts/get_maintainers.pl will return the maintainers
of a given file, *not* the list of maintainers if the file was a patch.

The output expected of add_maintainers is the latter, so drop the option
-f.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    This patch is candidate for Xen 4.11. Without it add_maintainers.pl
    will not return the correct maintainers. IHMO, it should be fixed
    ASAP to avoid promoting a broken scripts.
---
 scripts/add_maintainers.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Juergen Gross June 5, 2018, 6:15 p.m. | #1
On 05/06/18 18:39, Julien Grall wrote:
> The option -f of scripts/get_maintainers.pl will return the maintainers
> of a given file, *not* the list of maintainers if the file was a patch.
> 
> The output expected of add_maintainers is the latter, so drop the option
> -f.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen
Lars Kurth June 5, 2018, 6:26 p.m. | #2
On 05/06/2018, 19:15, "Juergen Gross" <jgross@suse.com> wrote:

    On 05/06/18 18:39, Julien Grall wrote:
    > The option -f of scripts/get_maintainers.pl will return the maintainers

    > of a given file, *not* the list of maintainers if the file was a patch.

    > 

    > The output expected of add_maintainers is the latter, so drop the option

    > -f.

    > 

    > Signed-off-by: Julien Grall <julien.grall@arm.com>

    
    Release-acked-by: Juergen Gross <jgross@suse.com>
 
Reviewed-by: Lars.kurth@citrix.com


This change in behaviour was mistakenly introduced in one of the later revisions (can't recall which), when we refactored the call to get_maintainers.pl

Lars
Julien Grall June 6, 2018, 10:06 a.m. | #3
Hi,

On 05/06/18 19:26, Lars Kurth wrote:
> 
> 
> On 05/06/2018, 19:15, "Juergen Gross" <jgross@suse.com> wrote:
> 
>      On 05/06/18 18:39, Julien Grall wrote:
>      > The option -f of scripts/get_maintainers.pl will return the maintainers
>      > of a given file, *not* the list of maintainers if the file was a patch.
>      >
>      > The output expected of add_maintainers is the latter, so drop the option
>      > -f.
>      >
>      > Signed-off-by: Julien Grall <julien.grall@arm.com>
>      
>      Release-acked-by: Juergen Gross <jgross@suse.com>
>   
> Reviewed-by: Lars.kurth@citrix.com
> 
> This change in behaviour was mistakenly introduced in one of the later revisions (can't recall which), when we refactored the call to get_maintainers.pl

Thank you! Other than that the script is working quite well and save me 
time to add all the CCs.

I will commit the patch once we branched.

Cheers,


> 
> Lars
>      
>
Julien Grall June 12, 2018, 10:44 a.m. | #4
On 06/06/18 11:06, Julien Grall wrote:
> Hi,
> 
> On 05/06/18 19:26, Lars Kurth wrote:
>>
>>
>> On 05/06/2018, 19:15, "Juergen Gross" <jgross@suse.com> wrote:
>>
>>      On 05/06/18 18:39, Julien Grall wrote:
>>      > The option -f of scripts/get_maintainers.pl will return the 
>> maintainers
>>      > of a given file, *not* the list of maintainers if the file was 
>> a patch.
>>      >
>>      > The output expected of add_maintainers is the latter, so drop 
>> the option
>>      > -f.
>>      >
>>      > Signed-off-by: Julien Grall <julien.grall@arm.com>
>>      Release-acked-by: Juergen Gross <jgross@suse.com>
>> Reviewed-by: Lars.kurth@citrix.com
>>
>> This change in behaviour was mistakenly introduced in one of the later 
>> revisions (can't recall which), when we refactored the call to 
>> get_maintainers.pl
> 
> Thank you! Other than that the script is working quite well and save me 
> time to add all the CCs.
> 
> I will commit the patch once we branched.

I was about to commit but I forgot that I would need a ack from someone 
in "THE REST".

Can I get one from either, Andrew, George, Ian, Jan, Tim, Wei, Stefano?

Cheers,
Ian Jackson June 12, 2018, 11:02 a.m. | #5
Julien Grall writes ("Re: [PATCH for-4.11] scripts/add_maintainers.pl: Don't call get_maintainers.pl with -f"):
> On 06/06/18 11:06, Julien Grall wrote:
> > I will commit the patch once we branched.
> 
> I was about to commit but I forgot that I would need a ack from someone 
> in "THE REST".
> 
> Can I get one from either, Andrew, George, Ian, Jan, Tim, Wei,
> Stefano?

Of course.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Patch

diff --git a/scripts/add_maintainers.pl b/scripts/add_maintainers.pl
index 99e4724112..09e9f6609f 100755
--- a/scripts/add_maintainers.pl
+++ b/scripts/add_maintainers.pl
@@ -420,7 +420,7 @@  sub ismailinglist ($) {
 sub getmaintainers ($$$) {
     my ($file, $rto, $rcc) = @_;
     my $fh;
-    open($fh, "-|", $get_maintainer, @get_maintainer_args, '-f', $file)
+    open($fh, "-|", $get_maintainer, @get_maintainer_args, $file)
         or die "Failed to open '$get_maintainer'\n";
     while(my $line = <$fh>) {
         chomp $line;