[Sugar-devel] Code Review process changes

[Bernie Innocenti]

El Fri, 30-04-2010 a las 10:49 +0200, Tomeu Vizoso escribió:
> > What's the problem with plain email reviews that we're trying to solve
> > with bug trackers and fancy review tools?
> 
> It's useful to have the patches tracked and related to the bug report.

Yes, but not all patches are related to bug reports. I'd like a fast
path for simple patches that took 1 minute to write and shouldn't take
more than 1 minute to review and apply.

With a smooth development process, such trivial patches dominate the
overall volume of patches. But if the overhead gets too high, they're
lost.


> I don't have time to waste discussing how our system could be
> completely different. After we changed to use the same system as
> Linux, someone would start complaining we should switch to Mozilla's.

Changing or refining the development process is as important as writing
code. Now we have a problem that code already written is stuck because
the current process is failing.

On IRC, you said you were in favor of doing reviews directly on the
mailing list. Everyone else agreed, so I this part of the change should
be already settled.


> We have a system modelled after GNOME's and it used to work when we
> had maintainers, and of course it works for the dozens of modules in
> GNOME and Freedesktop. Sure, we can make changes to adapt to the
> specifics of Sugar's community, but that request for change should
> come from the maintainers, who are the ones that will be most directly
> affected by them.

The difference between us and GNOME seems to be in the availability of
maintainers. We can go back to the GNOME model when we'll have solved
this issue, but at this time this model is clearly not working for us.


> > Can peers also approve patches?
> >
> > If so, then I think we've already solved our issue.
> 
> Well, we don't have enough peers, many of the listed them are not
> active any more.

Having unresponsive people listed as maintainers/peers creates a false
expectation.

Let's remove those that did not post to the list or Trac over the last 2
months. We can quickly add them back if they come back.



> > Being so tightly coupled, these 4 modules should probably share the same
> > set of maintainers and peers.
> 
> That doesn't match my actual experience maintaining Sugar. You are
> guessing about what some people have actually experienced, please stop
> doing that.

I'm not guessing. I've actually heard this complaint from several people
who shall speak up for themselves publicly if they want to. This is the
first item in Michael's experimental fork of Sugar lists as the first
item:

  * combines all six of the sugar, sugar-toolkit, sugar-base,
    sugar-artwork, sugar-datastore, and sugar-presence-service
    repos into a single repo [1],

So I'm certainly the only one who thinks that the current shredding of
Sugar into 6 projects sucks. I'd be curious to know: who actually likes
it this way and why?


> > Hopefully we'll grow fresh new full-time maintainers from today's
> > newbies, but only if we make it possible for them to contribute now.
> 
> I'm not interested in discussing changes to the system as long as we
> have Sugar in such unmaintained state. I have tried at least a dozen
> of times to start a discussion on this resourcing problem and have
> been ignored.

Probably nobody knows what to answer! Free software projects like us
often have to get going with the resources that happen to become
available.

I'm convinced the current volunteers base would expand dramatically if
we'd let them contribute without demanding them full-time commitment.


> From my POV, changing the system to depend on less on maintainers is
> just side stepping the actual problem: nobody wants to do the boring
> maintenance work.
>
> I'm going to start one more thread about it and it will be my last try
> to get the SLs community to care about maintenance.

I'm discussing with a few people to see if we could fill-in the gaps.
Meanwhile, what shall we do? Do we halt development and give up?

What I'm proposing would solve our most urgent issue without making it
harder to find maintainers later on. Actually, it would make it a lot
easier.

We should at least try it before giving up.



> Giving more visibility to the review queue has nothing to do with
> where patches are posted and where the review happens.

It has all to do with it! Patches in trac have been ignored for months,
while all patches posted to this list immediately generated threads with
interesting ideas. The difference is so evident that there should be no
doubt about what's working best.

The only thing that's missing now is giving the *current* reviewers also
the authority to approve patches. We can call them "maintainers" or
"peers" if you like.


> No, as I wrote before, people approving patches should be those who
> are going to be taking responsibility on maintaining the new code and
> also those who know what is a good patch in that module's context,
> which means spending time triaging and bug fixing.

This is where we loose everyone imho. What you want is called a paid
employee!

I've seen the same mistake made by a manager (who shall remain
anonymous) who demanded that the community be working on fixing boring
bugs in the order prioritized by the company objectives. Guess what
happened? It was decided that the community was not being useful and
development became even more closed.

Community management simply doesn't work this way. At this time we have
plenty of contributors posting useful patches and fixing bugs that they
care about. Take it or loose it.

I understand your frustration, but management roles are the hardest to
fill in in free software projects. Luckily, community self organization
is often a pretty good replacement. Please, let's not assume a rigid
attitude of "do the boring work or go away", because it's a guaranteed
recipe for failure.

When someone steps up to maintain a module, we'll be more than glad to
re-adjust the commit policy accordingly. For example, sugar-jhbuild can
stay the way it is because Sascha seems to be a very responsive
maintainer.


> This is the main problem: we talk about work that needs to be done,
> but don't want to think about how we are going to resource it. I don't
> think that's healthy in an open community.

No, what's not healthy is insisting on a development model that requires
resources we currently don't have and thus loosing all the work that
*is* being done.


> Are you kidding? We have about 3 people who have done reviews in the
> recent past and those people accumulate lots of other Sugar and
> non-Sugar responsibilities. Do you know how many full-time maintainers
> has the linux kernel?

Do you know how many maintainers there were in the beginning? Only one.

Do you know how the Linux kernel got to have many more? By making it
damn easy to contribute. When I posted my first patch, I was not asked
to become a maintainer of all the files I modified.

Outsiders often look at the kernel development model and say: "ha, they
can do XYZ because they have so many resources to waste!" But they're
confusing the cause with the effect.

Other projects that were almost dying due to lack of contributors
suddenly got new life by simply switching to a bazaar model closer to
the kernel. One such example is Xorg. Cjb, who follows the its
development closely, could probably support my claim with some
statistical evidence.

I'm convinced the same would work in revitalizing Sugar's development.
Please, let us try. You won't regret it.

-- 
   // Bernie Innocenti - http://codewiz.org/
 \X/  Sugar Labs       - http://sugarlabs.org/