[Sugar-devel] Code Review process changes

[Tomeu Vizoso]

> Hi!
>
> Bernie, Tomeu and I had a nice discussion regarding the Code Review
> process on #sugar yesterday. To sum it up:
>
> Several contributors are hindered or even put off by the current
> process. It often takes more time to handle mere technicalities (save
> patch to file, create ticket in Trac, attach patch, wait for review,
> push) than it takes to fix the bug.

We talked before of using XML-RPC to automate it, what happened to that?

I'm using this for GNOME and Freedesktop projects and works very well:

http://git.fishsoup.net/man/git-bz.html

I also heard about a trac<->email gateway, any news?

> In addition there's a large backlog
> because reviews are currently handled only by module maintainers. The
> latter issue turned out to be at least partly due to misunderstandings
> about who "can" do reviews.

If we have such a thing as module peers is solely for helping out with reviews:

http://wiki.sugarlabs.org/go/Development_Team/Release/Modules

But we also should have more than one maintainer per module.

> We'd like to try a different approach that's used by many successful
> projects - both small and large ones.
> Patches are sent to sugar-devel for review. Every Sugar developer (*)
> can review patches (and multiple reviews are quite welcome). Since the
> number of developers with commit access is limited, we have a sufficient
> level of QA even without limiting who "can" do reviews.

This seems to imply that the sole purpose of reviews is QA, but I
think it has two more important purposes: transferring the burden of
maintenance and sharing best practices and conventions.

In practical terms, to me this means that reviews need to come from
those who can appreciate the actual impact on maintenance of a patch,
and that also have a direct interest in keeping the maintenance burden
low.

> There are a number of systems to track the status of patches sent to the
> list (e.g. Patchwork [1]), but as this adds complexity (and yet another
> system to maintain) again we'd like to try without at first.

I'm confused, how are these systems better than the patch review
report we used to have?

For those who weren't with us back then:

http://lists.sugarlabs.org/archive/sugar-devel/2008-July/006903.html

> Personal note:
> Instead of using a patch tracker, we could also ask patch submitters to
> file tickets at bugs.sugarlabs.org if there has been no review for, say,
> 3 days. This gives a streamlined process for most patches while still
> ensuring nothing gets lost.

So we need to look for patches in two places?

Regards,

Tomeu

>
> (*) We defined "Sugar developer" as "anybody who has made at least one
> change that entered mainline".
> [1] http://patchwork.ozlabs.org/
>
> CU Sascha