[Sugar-devel] code review (was Re: buddy tags)

[Tomeu Vizoso]

On Tue, Aug 4, 2009 at 15:23, Michael Stone<michael at laptop.org> wrote:
>
> Tomeu,
>
> I like your response but I'm surprised that you're confused about why your
> review process is getting bogged down, so I'll share my perspective in the
> hopes that you will find it helpful.
>
> In short, expecting the submitter to do all the work listed in your review
> process condemns many perfectly reasonable patches to oblivion and
> discourages
> contributors from submitting. [1]
> What you should do instead is to build community around each patch *by
> directly
> asking specific individuals* to step in. [2]
>
> (Bernie, Sascha, Chris Ball, and I would all seem like good victims for code
> review work.) [3]
>
> This way, you get more people involved in each patch, familiar with its
> ideas,
> and comfortable supporting one another. That's how you ramp up the volume of
> contribution. [4]

I don't think we just need more reviewers. I think that what we need
is more module maintainers and peers. Those people will make reviews
because they are the people who are going to spend their time
debugging and modifying the submitted code.

The purpose of a review is not evaluating the coding skills of the
submitter, but rather:

- make sure that the code is maintainable _by the current maintainers_,

- increase understanding of the community of what the current
maintainers think they can maintain.

The QA aspect of it is also very important, but isn't what makes the
current reviews more painful than they should be. When Marco, Simon
and me reviewed each others code, we were spotting flaws, plain bugs
and design decisions that could be improved. We all learned and were
happy that our code was better because of other people's reviews.

But the reviews we are doing today is more about explaining in a very
slow, frustrating and inefficient way that the code that gets into the
repository needs to have some qualities that are subjective in some
way.

I hope we can improve a bit by making more explicit which are those
qualities, but we still need more people maintaining if we are to
successfully accept significant code contributions.

Regards,

Tomeu
- Hide quoted text -

> Regards,
>
> Michael
>
>
>
> Notes:
>
> [1]: Drucker famously wrote that
>  Management is about human beings. Its task is to make people capable of
>  joint performance, to make their strengths effective and their weaknesses
>  irrelevant.
>
> We aren't doing that very well here, which is why your process is failing.
> (He also had some good advice on how to do it: see, e.g., "The Essential
> Drucker", ISBN 978-0066210872).
>
> [2]: The individuals can obviously say no, but I have found that we're all
> /far/ more likely to do things when someone we respect asks us, personally,
> to do them.
>
> [3]: We're just good guinea pigs because we're all friends of yours who are
> familiar with the Sugar code base, who like making lots of different small
> technical contributions, and who I know to be capable of giving a decent
> code
> review.
>
> [4]: People like to feel good and helping your friends out in small ways
> often
> feels really good. You should take more advantage of this phenomenon. :)
>
>
>
> Other notes:
>
>  * This strategy also works well for testing and deployment feedback; c.f.
>   Friends in Testing, NZ-testing, and the relationships that Greg formed
> with
>   people he worked with, like Pablo Flores.
>
>  * Another good name for this strategy is "providing good customer service".
>   You just have to realize that SL coders are customers of SL who are
> trading
>   time and frustration for satisfaction and experience.
>
>  * Does anyone need more examples, clarifications, or advice?
>