[Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Michael S. Tsirkin]

We have a bunch of modules in "Odd fixes"
status, scripts/get_maintainer.pl ignores that.

Reported-by: Gerd Hoffmann <kraxel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 scripts/get_maintainer.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 38334de..d5eee8c 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -514,7 +514,7 @@ sub range_is_maintained {
 	    my $type = $1;
 	    my $value = $2;
 	    if ($type eq 'S') {
-		if ($value =~ /(maintain|support)/i) {
+		if ($value =~ /(maintain|support|fix)/i) {
 		    return 1;
 		}
 	    }
-- 
MST

Re: [Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Don Slutz]

On 10/21/14 07:35, Michael S. Tsirkin wrote:
> We have a bunch of modules in "Odd fixes"
> status, scripts/get_maintainer.pl ignores that.
>
> Reported-by: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   scripts/get_maintainer.pl | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 38334de..d5eee8c 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -514,7 +514,7 @@ sub range_is_maintained {
>   	    my $type = $1;
>   	    my $value = $2;
>   	    if ($type eq 'S') {
> -		if ($value =~ /(maintain|support)/i) {
> +		if ($value =~ /(maintain|support|fix)/i) {
>   		    return 1;
>   		}
>   	    }

Looks good to me.

Reviewed-by: Don Slutz <dslutz@verizon.com>

    -Don Slutz

Re: [Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Michael S. Tsirkin]

On Tue, Oct 21, 2014 at 02:35:09PM +0300, Michael S. Tsirkin wrote:
> We have a bunch of modules in "Odd fixes"
> status, scripts/get_maintainer.pl ignores that.
> 
> Reported-by: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Actually, it does not ignore that.
What it does it list status as "odd fixer".

So this boils down to the fact that we use
"odd fixer" incorrectly.

I will apply this for now but maybe we should
change MAINTAINERS switching all "Odd fixes" to
"Maintained" and then revert this patch.

Thoughts?

> ---
>  scripts/get_maintainer.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 38334de..d5eee8c 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -514,7 +514,7 @@ sub range_is_maintained {
>  	    my $type = $1;
>  	    my $value = $2;
>  	    if ($type eq 'S') {
> -		if ($value =~ /(maintain|support)/i) {
> +		if ($value =~ /(maintain|support|fix)/i) {
>  		    return 1;
>  		}
>  	    }
> -- 
> MST

Re: [Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Paolo Bonzini]

On 10/22/2014 09:22 AM, Michael S. Tsirkin wrote:
> Actually, it does not ignore that.
> What it does it list status as "odd fixer".
> 
> So this boils down to the fact that we use
> "odd fixer" incorrectly.
> 
> I will apply this for now but maybe we should
> change MAINTAINERS switching all "Odd fixes" to
> "Maintained" and then revert this patch.
> 
> Thoughts?

To me "odd fixes" means I may help shepherding your patches into the
tree, but I would rather revert a buggy patch than fix it.  And unless
they're trivial or egregious, I would not spend much time on bug
reports.  It just sets expectations right.

Paolo

Re: [Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Tue, Oct 21, 2014 at 02:35:09PM +0300, Michael S. Tsirkin wrote:
>> We have a bunch of modules in "Odd fixes"
>> status, scripts/get_maintainer.pl ignores that.
>> 
>> Reported-by: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> Actually, it does not ignore that.
> What it does it list status as "odd fixer".

Like this:

    $ scripts/get_maintainer.pl -f --no-git-fallback hw/ide/core.c 
    Kevin Wolf <kwolf@redhat.com> (odd fixer:IDE)
    Stefan Hajnoczi <stefanha@redhat.com> (odd fixer:IDE)

What's wrong with that?

> So this boils down to the fact that we use
> "odd fixer" incorrectly.
>
> I will apply this for now but maybe we should
> change MAINTAINERS switching all "Odd fixes" to
> "Maintained" and then revert this patch.
>
> Thoughts?

What exactly does this patch fix?

Re: [Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Paolo Bonzini]

On 10/22/2014 09:47 AM, Markus Armbruster wrote:
> Like this:
> 
>     $ scripts/get_maintainer.pl -f --no-git-fallback hw/ide/core.c 
>     Kevin Wolf <kwolf@redhat.com> (odd fixer:IDE)
>     Stefan Hajnoczi <stefanha@redhat.com> (odd fixer:IDE)
> 
> What's wrong with that?

Michael's patch fixes the case where you do _not_ have --no-git-fallback:

    $ scripts/get_maintainer.pl -f hw/ide/core.c 
    Kevin Wolf <kwolf@redhat.com> (odd fixer:IDE,commit_signer:6/27=22%)
    Stefan Hajnoczi <stefanha@redhat.com> (odd fixer:IDE,commit_signer:18/27=67%)
    John Snow <jsnow@redhat.com> (commit_signer:13/27=48%)
    Paolo Bonzini <pbonzini@redhat.com> (commit_signer:11/27=41%)
    Fam Zheng <famz@redhat.com> (commit_signer:4/27=15%)

With the patch:

    $ scripts/get_maintainer.pl -f hw/ide/core.c 
    Kevin Wolf <kwolf@redhat.com> (odd fixer:IDE)
    Stefan Hajnoczi <stefanha@redhat.com> (odd fixer:IDE)

which behaves the same as:

    $ scripts/get_maintainer.pl -f block/qed.c
    Kevin Wolf <kwolf@redhat.com> (supporter:Block)
    Stefan Hajnoczi <stefanha@redhat.com> (supporter:Block)

Paolo

Re: [Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 10/22/2014 09:47 AM, Markus Armbruster wrote:
>> Like this:
>> 
>>     $ scripts/get_maintainer.pl -f --no-git-fallback hw/ide/core.c 
>>     Kevin Wolf <kwolf@redhat.com> (odd fixer:IDE)
>>     Stefan Hajnoczi <stefanha@redhat.com> (odd fixer:IDE)
>> 
>> What's wrong with that?
>
> Michael's patch fixes the case where you do _not_ have --no-git-fallback:
>
>     $ scripts/get_maintainer.pl -f hw/ide/core.c 
>     Kevin Wolf <kwolf@redhat.com> (odd fixer:IDE,commit_signer:6/27=22%)
>     Stefan Hajnoczi <stefanha@redhat.com> (odd
> fixer:IDE,commit_signer:18/27=67%)
>     John Snow <jsnow@redhat.com> (commit_signer:13/27=48%)
>     Paolo Bonzini <pbonzini@redhat.com> (commit_signer:11/27=41%)
>     Fam Zheng <famz@redhat.com> (commit_signer:4/27=15%)
>
> With the patch:
>
>     $ scripts/get_maintainer.pl -f hw/ide/core.c 
>     Kevin Wolf <kwolf@redhat.com> (odd fixer:IDE)
>     Stefan Hajnoczi <stefanha@redhat.com> (odd fixer:IDE)
>
> which behaves the same as:
>
>     $ scripts/get_maintainer.pl -f block/qed.c
>     Kevin Wolf <kwolf@redhat.com> (supporter:Block)
>     Stefan Hajnoczi <stefanha@redhat.com> (supporter:Block)

Got it, thanks!

Shows how automatic --no-git-fallback has become for me...

>> > So this boils down to the fact that we use
>> > "odd fixer" incorrectly.
>> >
>> > I will apply this for now but maybe we should
>> > change MAINTAINERS switching all "Odd fixes" to
>> > "Maintained" and then revert this patch.
>> >
>> > Thoughts?
>> 
>> What exactly does this patch fix?

No objections to applying the patch.

Re: [Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Paolo Bonzini]

On 10/22/2014 10:08 AM, Michael S. Tsirkin wrote:
>> > 
>> > To me "odd fixes" means I may help shepherding your patches into the
>> > tree, but I would rather revert a buggy patch than fix it.
> OK but this is not what MAINTAINERS says:
> 
> 
>            Odd Fixes:   It has a maintainer but they don't have time to do
>                         much other than throw the odd patch in. See below.
> 
> 
> I don't know what "See below" refers to.
> But it does seem to imply maintainers don't have time to
> review patches, so at some level, enabling fallback in
> this case seems to make sense.

To me it means that I have time to review patches, but not to do my own
work on it.

Paolo

Re: [Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Michael S. Tsirkin]

On Wed, Oct 22, 2014 at 09:45:13AM +0200, Paolo Bonzini wrote:
> On 10/22/2014 09:22 AM, Michael S. Tsirkin wrote:
> > Actually, it does not ignore that.
> > What it does it list status as "odd fixer".
> > 
> > So this boils down to the fact that we use
> > "odd fixer" incorrectly.
> > 
> > I will apply this for now but maybe we should
> > change MAINTAINERS switching all "Odd fixes" to
> > "Maintained" and then revert this patch.
> > 
> > Thoughts?
> 
> To me "odd fixes" means I may help shepherding your patches into the
> tree, but I would rather revert a buggy patch than fix it.

OK but this is not what MAINTAINERS says:


           Odd Fixes:   It has a maintainer but they don't have time to do
                        much other than throw the odd patch in. See below.


I don't know what "See below" refers to.
But it does seem to imply maintainers don't have time to
review patches, so at some level, enabling fallback in
this case seems to make sense.



> they're trivial or egregious, I would not spend much time on bug
> reports.  It just sets expectations right.
> 
> Paolo

Re: [Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Michael S. Tsirkin]

On Wed, Oct 22, 2014 at 10:07:52AM +0200, Paolo Bonzini wrote:
> 
> 
> On 10/22/2014 10:08 AM, Michael S. Tsirkin wrote:
> >> > 
> >> > To me "odd fixes" means I may help shepherding your patches into the
> >> > tree, but I would rather revert a buggy patch than fix it.
> > OK but this is not what MAINTAINERS says:
> > 
> > 
> >            Odd Fixes:   It has a maintainer but they don't have time to do
> >                         much other than throw the odd patch in. See below.
> > 
> > 
> > I don't know what "See below" refers to.
> > But it does seem to imply maintainers don't have time to
> > review patches, so at some level, enabling fallback in
> > this case seems to make sense.
> 
> To me it means that I have time to review patches, but not to do my own
> work on it.
> 
> Paolo

Intersting.
review is "other than throw the odd patch in" so it's hardly
the literal meaning of this text.

-- 
MST

Re: [Qemu-devel] [PATCH] scripts/get_maintainer.pl: allow "odd fixes"

[Paolo Bonzini]

On 10/22/2014 10:13 AM, Michael S. Tsirkin wrote:
> > To me it means that I have time to review patches, but not to do my own
> > work on it.
> 
> Intersting.
> review is "other than throw the odd patch in" so it's hardly
> the literal meaning of this text.

Well, review is just the prerequisite/necessary evil for throwing the
odd patch in. :)

Paolo