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

[From: Andrew Flegg]

On Sun,   6 Feb 2011, 15:51:50 GMT, Alberto Mardegan <mardy at users.sourceforge.net> wrote:
>
> 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

As to be expected, as this is a *testing* release to start defining processes and get more involved. For what it's worth, the following is a less noisy query:

    http://j.mp/communityssu-bugs 
 
> 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.

I don't think it's critical at all. Certainly not compared with the community-ssu-enabler postinst bug before the announcement which required editing /var/lib/dpkg/community-ssu-enabler.postinst to do the upgrade.

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

This was my bad: the fix was in gitorious but not in the repo. However, I'd say this is the purpose of having a testing repo; and the clear warnings both at install time and on the Community_SSU page which say that currently it's only for those "willing to risk a reflash."
 
> 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.

My own thought is that a comprehensive set of test scripts can cover the main bases before something gets pushed to the stable repo. These can be crowdsourced at both writing and testing:

    http://wiki.maemo.org/Community_SSU/QA
 
> PROPOSAL: MAILING LIST FOR CODE REVIEW

I don't think this is the best approach though. There are many problems to overcome:

  * It throws away the streamlined workflow supported
    by gitorious.org and its "merge request" functionality.
  * Who would volunteer to review the code? Currently 
    we have a surfeit of people working on the code of
    the CSSU, not a surplus. Given they're doing it in their
    spare time, drudgerous formal code review is not going
    to be high on their list.
  * In my professional experience, monolithic code reviews
    are rubbish at detecting bugs. One certainly wouldn't've 
    caught #11813.

Having said that, doing something informally should be fine. Gitorious offers "watch" and (IIRC) RSS functionality. If you, or anyone else, wanted to watch the commits and provide comments on maemo-developers, I think that'd be very useful.

If you were doing this, you could describe "how to reactively review code" under http://wiki.maemo.org/Community_SSU/Development (and/or .../QA#Process)

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

I thought the wiki pages did. However, it's a wiki, feel free to iteratively improve it!

Thanks in advance,

Andrew

-- 
Andrew Flegg -- mailto:andrew at bleb.org   |   http://www.bleb.org/
Maemo Community Council member