[Development] Maintainer "TrustMes"
Thiago Macieira
One of the duties of a maintainer is to ensure that every contribution to the
code he/she maintains is being reviewed. That is, if no one else approves or
rejects, the maintainer has the duty to make that happen. I've done this in
the past and approved based on +1 given by other people, or by reviewing stuff
myself.
What are we supposed to do when no one else approves or rejects a commit that
we created ourselves? Or worse, when no one reviews at all?
My question applies mostly to QtDBus, since I don't expect most people will
know anything about that module. I've (ab)used Stephen's goodwill to review
simple things, but I don't expect him to understand the message delivery path
for example.
What is the suggested procedure?
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Intel Sweden AB - Registration Number: 556189-6027
Knarrarnäsgatan 15, 164 40 Kista, Stockholm, Sweden
Atlant Schmidt
Does the project maintain a list of "Subject Matter Experts"
from which 1) volunteers could be solicited or 2) reviewers
could be "drafted"? If not, *SHOULD* the project maintain
such a list?
Otherwise, it's as we saw in "T2"...
http://noffload.net/uploader/files/1/term2/t2-%28498%29.jpg
"Trust me!"
Atlant
Hello
This e-mail and the information, including any attachments, it contains are intended to be a confidential communication only to the person or entity to whom it is addressed and may contain information that is privileged. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender and destroy the original message.
Thank you.
Please consider the environment before printing this email.
_______________________________________________
Development mailing list
Devel...@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Stephen Kelly
On Monday, April 02, 2012 10:22:12 Thiago Macieira wrote:
> Hello
>
> One of the duties of a maintainer is to ensure that every contribution to
> the code he/she maintains is being reviewed. That is, if no one else
> approves or rejects, the maintainer has the duty to make that happen. I've
> done this in the past and approved based on +1 given by other people, or by
> reviewing stuff myself.
>
> What are we supposed to do when no one else approves or rejects a commit
> that we created ourselves? Or worse, when no one reviews at all?
I have had the same problem sometimes.
(hint: http://codereview.qt-project.org/#change,21169)
I've been lucky enough that ogoffart can usually review them.
>
> My question applies mostly to QtDBus, since I don't expect most people will
> know anything about that module. I've (ab)used Stephen's goodwill to review
> simple things, but I don't expect him to understand the message delivery
> path for example.
I plan to look into that stuff soon this week.
>
> What is the suggested procedure?
IIRC, you need to make sure someone is educated enough about the stuff to give a +1, and then you can give a +2. I think that was documented in the procedures.
I do that all the time with the CMake stuff, because the people who can review them are not approvers.
What to do when that doesn't work I'm not sure. I guess it's a responsibility of other approvers and maintainers to try to review such things if you ask them to in such situations.
Thanks,
--
Stephen Kelly <stephe...@kdab.com> | Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090
KDAB - Qt Experts - Platform-Independent Software Solutions
Thiago Macieira
> Thiago:
>
> Does the project maintain a list of "Subject Matter Experts"
> from which 1) volunteers could be solicited or 2) reviewers
> could be "drafted"? If not, *SHOULD* the project maintain
> such a list?
Yes and yes.
That doesn't mean it happens or it can be done. I've been the sole developer
of QtDBus since 2006. The previous developer (HarryF) has long since stopped
looking at it, even though he's still around. In general, the amount of work
in small enough that one developer is enough to maintain it.
If no one else wants to learn about it, there's little I can do about it. I'm
no one's boss. But I still have a responsibility to ensure all changes are
reviewed, including mine.
Thiago Macieira
> > My question applies mostly to QtDBus, since I don't expect most people
> > will
> > know anything about that module. I've (ab)used Stephen's goodwill to
> > review
> > simple things, but I don't expect him to understand the message delivery
> > path for example.
>
> I plan to look into that stuff soon this week.
Thanks.
> > What is the suggested procedure?
>
> IIRC, you need to make sure someone is educated enough about the stuff to
> give a +1, and then you can give a +2. I think that was documented in the
> procedures.
I've done that with Andy and with Peppe (QRegExp and QRegularExpression,
respectively). That's fine.
It doesn't help me with QtDBus since there aren't people educated enough about
them, other than me. That means the module has a bus factor close to 1, which
is bad though.
> What to do when that doesn't work I'm not sure. I guess it's a
> responsibility of other approvers and maintainers to try to review such
> things if you ask them to in such situations.
And I don't want to overburden Lars with these things. And if I were to ask
another maintainer, say Gunnar, what can I expect of him? I really doubt he's
ever seen the codebase.
I also don't want to overburden you. Your helping me with one commit doesn't
make you an expert in the matter.
Oswald Buddenhagen
> What is the suggested procedure?
>
there is no approved way for bypassing even a superficial review. hardly
a problem - there is always somebody on irc available for a quick sanity
check.
João Abecasis
On 2. apr. 2012, at 15.22, ext Thiago Macieira wrote:
> One of the duties of a maintainer is to ensure that every contribution to the
> code he/she maintains is being reviewed. That is, if no one else approves or
> rejects, the maintainer has the duty to make that happen. I've done this in
> the past and approved based on +1 given by other people, or by reviewing stuff
> myself.
>
> What are we supposed to do when no one else approves or rejects a commit that
> we created ourselves? Or worse, when no one reviews at all?
>
> My question applies mostly to QtDBus, since I don't expect most people will
> know anything about that module. I've (ab)used Stephen's goodwill to review
> simple things, but I don't expect him to understand the message delivery path
> for example.
>
> What is the suggested procedure?
Code reviews, in general, are good. Even by people that don't necessarily know the code.
I think it would be fine to +2 yourself after you gathered +1s, if you can't get +2s in any other way.
Cheers,
João
marius.st...@nokia.com
> One of the duties of a maintainer is to ensure that every
> contribution to the code he/she maintains is being reviewed. That is,
> if no one else approves or rejects, the maintainer has the duty to
> make that happen. I've done this in the past and approved based on +1
> given by other people, or by reviewing stuff myself.
>
> What are we supposed to do when no one else approves or rejects a
> commit that we created ourselves? Or worse, when no one reviews at
> all?
>
> My question applies mostly to QtDBus, since I don't expect most
> people will know anything about that module. I've (ab)used Stephen's
> goodwill to review simple things, but I don't expect him to
> understand the message delivery path for example.
>
> What is the suggested procedure?
You don't have to have intimate knowledge about a module to review code
constructs against policies etc. And if there's no one else with
knowledge about a module, it's probably a good idea for someone to step
up and learn a little about that particular module. It's not good that
only 1 person knows a module, in case that person gets sick, get hit by
a bus (morbid I know, but still a possibilty) etc.
In any case, try to get someone to review at least the code syntax
parts, and see if you can persuade them to learning the technical bits
as well.
Last resort, the Chief Maintainer has the ultimate responsibility ;)
--
.marius
lorn....@nokia.com
On 02/04/2012, at 11:56 PM, ext Thiago Macieira wrote:
>
> If no one else wants to learn about it, there's little I can do about it. I'm
> no one's boss. But I still have a responsibility to ensure all changes are
> reviewed, including mine.
I know a bit about it, and won't mind helping out.
Lorn Potter
Senior Software Engineer, Core Enablers/QtSensors