[Sugar-devel] Code Review process changes

[Tomeu Vizoso]

On Fri, Apr 30, 2010 at 22:00, Bernie Innocenti <bernie at codewiz.org> wrote:
> 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.

Are you saying you cannot have a fast track that keeps patches
tracked? In GNOME and Freedesktop it takes a few seconds to submit a
patch for review to bugzilla:

$ git bz file my-product/some-component HEAD

>> 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.

I'm obviously ok with refining, I just don't have time energy nor time
to switch to a completely different system now.

For the sake of keeping some focus in this discussion, seems like you
are saying that we cannot get enough resources to keep a process in
which maintainers play a strong role. I actually think we can and I'm
proposing a plan in another thread to get those resources.

If SLs decides not to push for such a plan, then I would agree with
you in that we need to make without maintainers.

> 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.

It should be obvious, but just in case, we still need to agree on
specific changes to the process and need to update the documentation
in the wiki before we start using any different process than what we
have documented now.

>> 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.

Why it seems like that to you? For example, PyGObject is minimally
maintained right now and it's affecting our work on Python 3. People
are frustrated about it and we are discussing what to do, but I have
still to hear from anyone that the maintainer-based system must be
dropped. We are asking downstreams to contribute back with resources
so PyGObject is maintained and next week I will be flying to the
Ubuntu Developer Summit for that.

Resourcing maintenance is often a problem, but somehow, not all
projects think it's a good idea just to do without it. Maybe they have
a reason?

> 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.

Good luck with that experiment, I unfortunately won't have time to spend on it.

>> > 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.

Sounds good to me, any volunteer to go through the list and propose the changes?

>> > 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:

You are guessing because I doubt you have heard that complaint from
anybody who has _maintained_ sugar (please read my words with more
attention before replying).

>  * 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?

We need to split them conceptually because are different things.
Having each of those conceptual units in their own repo with their own
maintainers, bug tracker components helps to make this work division
more clear. It also happens to match the package split, which is
important in its own because we have a goal of running sugar
activities without having the sugar shell installed.

>> > 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!

Acknowledging that there's a problem is enough for a reply. But if
someone believed that maintainers weren't needed anyway, perhaps that
would be less of a problem to that person.

> Free software projects like us
> often have to get going with the resources that happen to become
> available.

I don't think that's the problem we face, but rather that we have been
offered the resources and we have failed to accept them because we
haven't growth from the stage of "showing we are able to do stuff" to
"we can talk to our partners and bring value to them".

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

Who is saying we shouldn't let volunteers contribute if they don't
commit full-time?

>> 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?

No, we keep developing but with a strong focus on stability and
integrating new maintainers.

> 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.

As I said above, I don't think that SLs is in need of experiments
right now, but rather is in need of focus and strategy. But SLs isn't
mine, if people think that we can do ok without maintainers, stable
processes, strategy or teams, that's fine. Good luck with that.

>> 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.

I meant it's orthogonal. When we used to have that weekly patch
report, we gave even more visibility to the queue without having the
actual patches there. Such a report can only be had if patches are
tracked in any way.

> 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!

Want it or not, the people deploying Sugar want to have some control
on it. They want it to be stable, they want to contain the features
they want, they want it to be ready for release at the appropriate
moment, etc. The maintainer figure is a pivotal element in the
mechanism through which downstreams influence their upstreams, and
specific provisions are also made so that volunteers can also
contribute.

It's true that downstreams haven't managed yet to make effective that
control, and the point I'm trying to make is that it's because SLs
hasn't focused enough in bringing them to work with us.

Your proposal of dropping maintainers is just working around that
failure of us and also will make SLs less interesting to our
downstreams if by the time we switch back to maintainers Sugar has
regressed strongly in some aspects.

> 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.

You are mixing two different things here. I want paid full-time
maintainers, but I don't see how that relates to volunteers fixing
bugs as prioritized by anyone.

> 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.

"Do the boring work or go away" is an extreme, and not relating at all
reward with responsibility is another. We need to strike a healthy
balance.

> 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.

So we just need to get more maintainers for the other modules and we
have the problem solved? Excellent, can you help me make that happen?
It's taking me much more time (and energy!) to reply your emails than
would take to organize deployment team meetings (or review patches).

>> 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.

Sincerely, you seem to reduce things to white or black just for the
sake of arguing. My proposal for bringing new maintainers addresses
the issue of the review queue, just in a different way.

>> 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.

You don't need to ask me, I don't own Sugar. For a year and a half I
have spent my energy and savings on making Sugar development
sustainable. If by some chance we have already reached that stage then
I will be happiest to call it done and step down as I'm quite tired.

IMO we are not there yet and I would advise to grow on the existing
team practices and experience rather than doing a radical experiment,
but it's not my call.

Regards,

Tomeu

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