[Sugar-devel] Bug tracking Vs Patch review

[Bernie Innocenti]

El Mon, 30-08-2010 a las 14:44 +0200, Tomeu Vizoso escribió:

> Went through http://wiki.sugarlabs.org/go/0.88/Platform_Components and
> googled for how each project prefer patches to be submitted:
>[...]

I probably contribute to very different type of projects, all of which
use lists for reviews: aros, autotools, binutils, cairo, coreboot, gcc,
git, glibc, gnash, linux, mesa, openwrt, xorg, uclinux, util-linux-ng.
Sorry for not providing links to the individual instruction pages -- I
double-checked the cases where I was unsure.

Perhaps low-level people like myself have different levels of tolerance
for overhead? But anyway, not long ago it seemed that, also among the
Sugar developers, there was a good level of consensus on moving towards
a lightweight process based on email... or at least allow both
submission methods, on the submitter's discretion.


> Didn't knew we had a design process, do you have a link?

We don't have a formalized process, nevertheless UI patches require a
prior discussion on the mailing-list with the Design Team, and often
need to be accompanied mock-ups describing the design. Making these
requires all the UI code to be already written, unless you're really
good at painting.

I have seen a number of otherwise good patches being rejected because
they had not gone through this (undocumented) process.


>  Anyway, most
> patches don't affect UI. For the ones that do, isn't it fair that the
> people involved in UI are given a chance to give their opinion? Same
> with giving a chance to deployers to give their opinions.

All opinions are good and welcome, but when one needs 4 different
opinions to converge before changing a line of code, the process gets
engulfed.

Sometimes UI designers come up with an idea that nobody would implement.
In other cases, programmers come up with UI changes that designers
reject. When you get both, the work needs to go through maintainer
approval of each affected module.

Add to this that many maintainers are unresponsive or vacant.

Very few patches make it to the end of this funnel. All the others
result in lost effort and frustrated contributors. We've seen some of
them complain, the majority simply disappears.


> > review process
> > (one per affected module).
> 
> See links above, seems to me that code review is an accepted practice
> in FOSS. Any reason we should be special?

Having some form of reviews is great, but we broke up Sugar in tightly
coupled modules, so that any non-trivial change often spans across 2 or
3. Typically it's sugar + sugar-toolkit + sugar-artwork.

Different people may be competent on different parts of the code, but
I'd rather have one incompetent decision maker rather than many
conflicting ones.


> > As a result, lot of good patches have got
> > stuck along the way, bitrotting in trac for 6 to 9 months.
> 
> Given what I wrote above, do you think it could be worthy considering
> other factors for this?

Sure, there might be multiple negative factors working at the same time.
Overhead of our workflow is just the most visible one.

Another one that concerns me very much that filing patches in the bug
trackers replaces the "many eyes" that should characterize FLOSS
development with just one pair of eyes... or even no eyes at all.

Very few people look daily at activity in the bug tracker and even fewer
participate in discussions of tickets filed by others. As a result, we
often learn about a patch only when (and if) it lands in git... when
it's too late for a public review.


> I don't find it more boring than the other projects I contribute to,
> but I have to recognize that the split between maintainers and
> contributors we have in Sugar drains my energy considerably, specially
> when excessive rhetoric is used to make points.

After a good night of sleep, I realize that my post sound too much like
a rant and too little like a constructive proposal. I apologize about
that.


> In any case, I'm sure we can make contributing much more welcoming and
> exciting, I'll like to hear proposals, but sadly they are likely to
> involve work and then someone needs to do it.

My proposal does not come in the form of yet another process because
what I'm suggesting is actually deregulation: move from a rule-based
system to case-by-case judgment by one maintainer who cares for the
entire codebase.

You say that there are not enough people to cover all the roles defined
by our processes, but the more people you add, the more development will
get engulfed.

Believe me on this: wherever I go, I meet plenty of people who say that
Sugar Labs is too hard to work with. Even people who are generally more
tolerant than me with rigid processes.


> We have bugs that get fixed even before a build has carried it, and we
> have bugs that are there since nearly day zero. How is that news? Try
> browsing through the bug trackers linked above and you will find the
> same.

The bad news is that we often introduce serious bugs that would be
easily spotted by a glance at the patch and yet they manage to get
through all our complicated processes.

One example. For the last 2 weeks, code in jhbuild has been crashing on
startup for several people (anish, tch, myself) on several distros
(Fedora 13, Fedora 14, Ubuntu Karmic). Regardless, we proceeded to
release 0.89.5 tarballs that downstreams promptly turned into broken
packages (tested on Fedora).

Why would something like  happen in the face of all the steps we take to
ensure quality? This episode alone proves that no amount of rules and
processes can ever replace the minimum amount of coordination and
testing required to ensure that the software we ship does at least
start.

We seem to be missing the equivalent of a "product manager"... someone
who cares for the product as a whole. In a different occasion, Walter
noted the need for an "architect", but in a project like ours these are
very much the same thing, just with an emphasis on the present or on the
future.

In the Linux world, this is called a Linus. Not even 100 module
maintainers will ever fulfill this one vital role.


> Maybe you have been right since day zero of this discussion and we
> need to adopt the same processes the kernel has, but in any case
> changes are most probably going to require that someone does some work
> and we seem to have some difficulty there.

We merged a total of 355 patches in sugar over the last 365 months.
Exactly one patch per day. Over the same period, Linus merged 100 times
this number of patches, meaning that maintaining Sugar should be
feasible even as a part-time job.

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