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

[From: Mohammad Abu-Garbeyyeh]

Hi,

On 02/06/2011 05:51 PM, Alberto Mardegan wrote:
> (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.
If you look at commit dates and time, you'll see that the bug was fixed 
before the SSU was released, but it wasn't pushed to the SSU before 
Jaffa made the release.
>
> 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 (?).
That commit is Matan's patch appled to the latest git repo of 
hildon-desktop, you can find the patch here:
http://my.svgalib.org/770/n900/hd-2.2.141/mhd31.diff (Thanks Matan).
Since .diff files don't hold git commit info, I'm not sure how else that 
patch could be applied.

The second commit is one to blame on hildon-desktop's makefile not 
cleaning up properly, so it was left over after the source was built.
>
> 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. :-)
You're overlooking the fact that this is community-testing which is not 
considered stable. This basically is similar to -devel, you're enabling 
it at your own risk, what's considered stable in community-testing will 
be pushed to the stable repository, similar to what's done with the 
Diablo CSSU.

[snip]
> 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.
Making it harder to enable won't do a thing, we've seen how many users 
have -devel enabled when they don't know how to fix what it can break.
>
>
> Ciao,
>   Alberto
>


-- 
Regards,
Mohammad Abu-Garbeyyeh