[maemo-developers] Proposal: code review process for community SSU

[From: Alberto Mardegan]

(note: follow-up set to maemo-developers ML)


Hi all!
   I think we all agree that we should try our best to deliver good 
software updates and avoid at all costs breaking any existing features.


CURRENT SITUATION
=================
Shortly after releasing the first community SSU, we already got a few bugs:
https://bugs.maemo.org/buglist.cgi?query_format=advanced&product=Maemo%205%20Community%20SSU&classification=Extras

Of these, https://bugs.maemo.org/show_bug.cgi?id=11813 was rather 
critical, as we can see from the amount of duplicate bugs it got.
One one hand, it's excellent that the bug was fixed so promptly; but on 
the other hand, I think we should realize that the risk of completely 
breaking or ruining the user experience with a non well tested SSU 
update is real.

I ran "git log -p" on the hildon-desktop repository (in the Community 
SSU project) and looked at the patches. It's scary. Commit 
04725f6e6b261654d90fadeb2a2fc258a2ee3d28 consists of "26 files changed, 
848 insertions(+), 17 deletions(-)" and commit 
0fea52b0b7ce2c27054e08e3fa2160106c8eeea2, which is titled "modify 
changelog" is adding one new source file (?).

No matter how good the developers are, I don't feel at all confident 
about having the development of core components happening in IRC, 
possibly with just 2 or 4 eyes reviewing the code.


POSSIBLE SOLUTIONS, BRIEF
=========================
So, either we stop advertising the SSU repository, and on the contrary 
make it even harder to enable than extra-devel (because potentially it 
is *much more dangerous* than that!), or we think of some measures to 
minimize the risks of breakage.

I would vote for the second: having more people using the 
work-in-progress community SSU is not only beneficial for testing, but 
for its development too, because more people get their hands itchy with 
the desire of improving it. :-)


PROPOSAL: MAILING LIST FOR CODE REVIEW
======================================
Something like the LKML (Linux Kernel Mailing List): all patches are 
sent to the mailing-list before being merged into the master branch, and 
all subscribed people can comment, vote against the inclusion, or 
require some changes.
We could introduce some rules about the number of people who need to 
approve a patch before it goes in, or assign components to maintainers, 
but I'd suggest to keep things simple and avoid creating processes until 
we really need to.

We could use the already existing maemo-commits or maemo-developers 
mailing list. I would opt for the latter, because I don't expect a huge 
traffic of patches; besides, the maemo-developers list is read by a few 
of the Nokia employees who originally wrote that code, so this could be 
of a huge help.


CONCLUSION
==========
Please comment on the proposal. At the very least, we should immediately 
take the action of making the community SSU harder to enable and clearly 
state in the wiki pages that it's very high risk software.


Ciao,
   Alberto

-- 
http://blog.mardy.it <- geek in un lingua international!