I thought it took approval of two reviewers for a change to be accepted.
 That's not quite the same as using a pool of reviewers to ensure that
*someone* has time to review the change...

Re: "dedicated reviewers" -- Isn't supposed to be peer review?  If the
reviewers are dedicated, then you lose the peer part.  Also, it would take
more than two people full time to get the review done.  In my experience, a
proper review can take as long to do as writing the original code...

Of course, there's always the possibility that a change hasn't been
approved because while no one wants to vote it down, no one believes in the
change enough to put their reputation on the line for it.  :-\

I've notice several instances where someone not only submitted a change but
they also +1'ed as a reviewer.  While I admire their self confidence, I
think that practice defeats the purpose of the review.

> If they don’t know Bluetooth, WRT or the Linux kernel that’s OK.
> Engineers are supposed to be smart, and they can learn.

The same can be said of a manager.  Would you trust code reviewed by a
manager? My point is that subject matter expertise is important because if
the defects were obvious enough for a beginner to catch, they would not
have been left in the code in the first place!

FWIW!

Paul


Paul Hanchett
-------------------
Infotainment Engineer
MSX on behalf of Jaguar Land Rover
One World Trade Center, 121 Southwest Salmon Street, 11th Floor, Portland,
Oregon, 97204

Email: [email protected]
-------------------

Business Details:
Jaguar Land Rover Limited
Registered Office: Abbey Road, Whitley, Coventry CV3 4LF
Registered in England No: 1672070


On Fri, Jan 31, 2014 at 8:33 AM, Schaufler, Casey <[email protected]
> wrote:

>  The correct number of reviewers is 2. Not per project, total.
>
>
>
> If you dedicate 2 people to the review process (it’s possible but unlikely
> that they would be able to do anything else while so encumbered) you get
> accountability and timely response. If they don’t know Bluetooth, WRT or
> the Linux kernel that’s OK. Engineers are supposed to be smart, and they
> can learn. If they have questions, they can ask them.
>
>
>
>
>
> *From:* [email protected] [mailto:[email protected]] *On
> Behalf Of *Hanchett, Paul
> *Sent:* Friday, January 31, 2014 8:01 AM
> *To:* Łukasz Stelmach
>
> *Cc:* Philippe Coval; [email protected]
> *Subject:* Re: [Dev] State of Pending changes , how to enhance workflow ?
> ( Was Dear Maintainers )
>
>
>
> As a Management Principle, a smaller group tends to act with more personal
> accountablility than does a larger one.  One management guru says it thus
> "When everyone is responsible <for something>, then no one is."
>
>
>
> I'd suggest that even 10 reviewers might be too many.
>
>
>
> I would also think that a reviewer needs to be a SME (subject matter
> expert); I'm not sure how you'd bring that into the mix, but it could be
> important from a quality control perspective.
>
>
>
> Not sure that's worth even $0.02, but it's what I have to contribute.  <g>
>
>
>
> Paul
>
>
>
>
>
> Paul Hanchett
> -------------------
> Infotainment Engineer
> MSX on behalf of Jaguar Land Rover
> One World Trade Center, 121 Southwest Salmon Street, 11th Floor, Portland,
> Oregon, 97204
>
> Email: [email protected]
> -------------------
>
> Business Details:
> Jaguar Land Rover Limited
> Registered Office: Abbey Road, Whitley, Coventry CV3 4LF
>
> Registered in England No: 1672070
>
>
>
> On Fri, Jan 31, 2014 at 1:25 AM, Łukasz Stelmach <[email protected]>
> wrote:
>
>  It was <2014-01-30 czw 23:53>, when Thiago Macieira wrote:
> > On quinta-feira, 30 de janeiro de 2014 21:54:20, Patrick Ohly wrote:
> >> On Thu, 2014-01-30 at 10:48 -0800, Thiago Macieira wrote:
> >> > Ensuring that everything got reviewed is the responsibility of the
> >> > maintainer. But it's the *interest* of the contributor, so the
> >> > contributor should pay attention to the state of the contribution.
> >> >
> >> > If a contribution goes unreviewed after a while, ping the reviewers.
> >>
> >> That's impractical when the list of reviewers is larger than a handful.
> >> Who should get contacted? Randomly? Chances are high that the ones not
> >> doing any active work on the package will get pinged, annoying them
> >> further and causing additional delays.
> >
> > Just write a new comment saying "ping".
> >
> > Any of the reviewers is capable of approving the change. The question is
> only
> > whether they feel confident in doing so.
> >
> >> What's your take on those packages which currently have more than ten
> >> reviewers?
> >
> > Having that many reviewers is great! That means more people to look at
> > the changes, to find potential issues, offer advice as well as to
> > provide round-the- clock ability to review things. Having too few
> > reviewers (or none) is a bad thing -- for example, for my changes in
> > QtCore, very often no one can review them, which means I need to seek
> > exceptions.
>
> On the other, from a reviewer's perspective, hand having too many
> packages to review is bad.
>
>
> > The problem we have in Tizen is not that we have too few
> > reviewers. It's that each reviewer says "there are others, they'll
> > review" and no one ends up doing it.
> >
> > I would have preferred that the Cc list for each contribution have no
> > more than 10 people. More than that, we've got the effect I mentioned.
>
> That is exactly the point. And you also need to make sure that each of
> thos 10 people won't get notifications for more than N (IMHO N ~ 3 and I
> can negotiate that) repositories.
>
> I admit I send (almost) all the gerrit messages to bit bucket (manually
> but still) and treat them (their presence in my INBOX) only as a
> reminder to go to Gerrit and take a closer look. Then I also have a
> script that removes me from lists of reviewers for changes of several
> packages I know I can't help with. Only from time to time I take a look
> at random one of such changes at give my comments (most of the time I
> hate commit messages).
>
> As I wrote in December the compromise would be:
>
> - set up lists of *notified* pople for each project separate from the
>   "domain groups",
>
> - allow all members of the "domain groups" to excersise their
>   permissions in all repositories of the domain.
>
> However risky the second point may look, it will help in many simple
> situations where one member of a team (there is currently six in mine)
> is sick go for holiday and the others can take his duties.
>
> --
> Łukasz Stelmach
> Samsung R&D Institute Poland
> Samsung Electronics
>
> _______________________________________________
> Dev mailing list
> [email protected]
> https://lists.tizen.org/listinfo/dev
>
>
>
_______________________________________________
Dev mailing list
[email protected]
https://lists.tizen.org/listinfo/dev

Reply via email to