| Message ID | 1413891300-23369-1-git-send-email-mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
"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?
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
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.
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
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
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.
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
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; } }
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(-)