mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <benjamin.mah...@gmail.com>
Subject Re: Shepherding on ExternalContainerizer
Date Tue, 25 Mar 2014 06:47:02 GMT
Hey Till,

We want to foster a healthy review culture, and so, as you observed, we
thought we would try out the notion of having a "shepherd" for each review.

In the past we've had some reviews stagnate because there was no clear
accountability for getting it committed. Meaning, various committers would
be included in the 'Reviewers' and each would provide feedback
independently, but there was no single person accountable for "shepherding"
the change to a shippable state, and ultimately committing it.

We've also had issues with having a lot of lower value reviews crowding out
higher value reviews. Often these lower value reviews are things like
cleanup, refactoring, etc, which tend to be easier to review. Shepherding
doesn't address this as directly, but it is also an effort to ensure we
balance low value changes (technical debt, refactoring, cleanup, etc) with
higher value changes (features, bug fixes, etc) via shepherd assignment.

This is why we've been trying out the "shepherd" concept.

Related to this (and *not* related to your changes Till :)), I would
encourage two behaviors from "reviewees" to ameliorate the situation:

1. Please be cognizant of the fact that reviewing tends to be a bottleneck
and that reviewer time is currently at a premium. This means, please be
very thorough in your work and also look over your patches before sending
them out. This saves your time (faster reviews) and reviewers' time (fewer
comments needed). Feel free to reach out for feedback before sending out
reviews as well (if feasible).

2. Also, be cognizant of the fact that we need to balance low and high
priority reviews. Sometimes we don't have time to review low value cleanup
work when there are a lot of things in flight. For example, I have a bunch
of old cleanup patches from when we need to get more important things
committed, and I know Vinod has old cleanup patches like this as well.

This all being said, the external containerizer is high value and should
definitely be getting reviews. I will take some time to go over your
changes later this week with Ian, when I'll be free from a deadline ;). We
can help "pair shepherd" your changes.

Ben


On Mon, Mar 24, 2014 at 4:32 PM, Till Toenshoff <toenshoff@me.com> wrote:

> Dear Devs/Committers,
>
> after having developed the ExternalContainerizer, I am now obviously eager
> to get it committed. After receiving and addressing a couple of comments
> (thanks @all who commented - that helped a lot), I now am once again in a
> stage of waiting and keeping fingers crossed that my patch won't need
> rebasing before someone has a thorough look at it. I do appreciate and
> fully understand the fact that you committers are under heavy load.
>
> By experience and seeing some RR comments, I learned that there appears to
> be a new entity in our review process; a "shepherd". Sounds like a great
> idea, even though I am not entirely sure what that means in detail for
> Mesos. I guess that is something that makes sure that final commit
> decisions  are done by a single voice, preventing contradicting comments
> etc... Knowing that other projects actually demand the patch-submitter to ask
> for shepherding, I figured why not doing the same.
>
> For that ExternalContainerizer baby, I would kindly like to call out for a
> shepherd. Guessing that a shepherd needs to be a committer but also knowing
> that Ian is very deeply involved within containerizing, I would like to
> "nominate" Niklas as a committer in collaboration with Ian. Hope that makes
> sense and don't hesitate to tell me that this was not the right way to
> achieve shepherding.
>
> cheers!
> Till
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message