On 05/09/2018 03:11 PM, John Ferlan wrote:
> On 05/09/2018 04:57 AM, Daniel P. Berrangé wrote:
>> On Wed, May 09, 2018 at 10:46:19AM +0200, Jiri Denemark wrote:
>>> On Tue, May 08, 2018 at 14:07:31 +0100, Daniel P. Berrangé wrote:
>>>> Currently all patches are simply sent to the main libvirt development
>>>> mailing list. Sometimes individual developers are also CC'd but this is
>>>> typically the exception.
>>>> Libvirt does not follow a subsystem maintainer model, so there is no
>>>> notion of owners for the different areas of code, but there certainly
>>>> are people with high levels of expertize in specific areas.
>>>> This patch thus proposes pulling in QEMU's get_maintainer.pl script and
>>>> creating MAINTAINERS file to list who the experts are for specific
>>>> areas. Combined with git-pubish, this will help ensure that patches are
>>>> brought to the attention of people with direct expertize.
>>>> For example:
>>>> $ git show b04629b62934caa8786e73c3db985672422fc662 | \
>>>> Jim Fehlig <jfeh...@suse.com> (maintainer:libxl)
>>>> firstname.lastname@example.org (open list:All patches)
>>>> $ git show e7cb9c4e230c3c77e35e72334c261b5b0a2211c6 | \
>>>> Jiri Denemar <jdene...@redhat.com> (maintainer:CPU modelling)
>>> s/Denemar/Denemark/g :-)
>>>> email@example.com (open list:All patches)
>>>> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
>>> Anyway, I don't think we have such a high volume of patches that
>>> reviewers can't watch incoming mails for patches that touch an area they
>>> care about or are interested in. In other words, I don't mind this if
>>> it's optional, but I don't see a real benefit. If someone doesn't have
>>> time to review a particular patch, CCing them won't help anyway. And I'm
>>> afraid it could even cause others to not look at the patch since they
>>> would think the "maintainer" has to handle it.
>> Sure, I'm not claiming it magically makes time to review something, but we
>> do have a pretty high volume of incoming patches to track, making it easy
>> for things to get lost / missed. For people who work on libvirt only part
>> time, tracking all mails is a non-negligible undertaking.
> IMO, the volume of patches recently seems to be split between 3 things -
> libvirt proper, libvirt dbus, and "build related" with the latter two
> really being busy. I've personally assumed that the latter two based on
> their fairly quick review response time are "in good hands", I just mark
> them that way as part of my libvir-list patch work flow processing. That
> just leaves the review cycle time for libvirt specific patches.
>> The idea of CC'ing is that it alerts people to patches where they might have
>> a direct interest. Even if they don't have time to review, it gives them an
>> alert that someone is proposing something that might be relevant / impacting
>> on them.
> Perhaps, depends on your CC and libvir-list processing and filtering
> rules... If that mail ends up in my "Inbox" instead of "libvir-list"
> folder, then it's more time consuming for me as a regular reader to
> handle that and I may not notice it "right away" either.
This is a matter of e-mail filtering. For instance any patch that I'm
CCed on will end up in a separate folder different to INBOX (which also
depends on your work flow: some like to keep inbox empty, whereas I
"only" read e-mails and don't move them).
> Then there are those that don't like being CC'd - something you learn
> the more time you spend following the list. Unfortunately there are
> those that use CC just because someone has reviewed their patch
> previously or responded to a query - even though the next series or
> question has nothing to do with the previous.
Again, I'd say this is matter of proper filter set up.
> Another downside to MAINTAINERS is maintaining it. Even more painful if
> someone wants to change their email or something similar. Those that
> follow this list, do reviews, and have commit access have a general idea
> of where expertise lies. If I'm reviewing something in CPU or Migration
> and am not sure, I will either ask or wait for Jirka to have a looksee
> as well.
Well, that's only part of the problem. For instance, I've written QoS
and I'd like to review any patch against src/util/virnetdevbandwidth.c
(just read the comment in virNetDevBandwidthSet and you'll see how
complicated this feature is). And quite frankly, if I need to catch up
with the upstream list (e.g. after some vacation), I merely just skim
over patches, and if any of them has a reply I skip the whole thread.
That is, I don't read others reviews. But if I were CC'ed, I'd get
another e-mail that would end up in different folder (with much lower
traffic) and therefore it would get my attention.
Sure, it's matter of fine tuning - if there would be somebody CCed on
every patch than this does not make things any better. I view this patch
of Dan's as a way for us, "owners" of some features to be notified when
somebody tries to patch "our" code.
> While it's far from perfect, the current model seems to for the most
> part work. If a patch is missed, someone interested will ping on it. In
> the end, it's generally more of a "time" problem for those of us
> developing and reviewing/pushing.
Well, long gone are times when Eric was around and not only read every
e-mail on the list but also compared patches sent to the list with those
actually pushed. I honestly say, if any patch has a review (unless it
has subject that interests me) I skip the whole patch set and mark it as
tl;dr I like this patch.
libvir-list mailing list