Tuesday, 28 March 2017

Re: SRU quality and preventing regressions


On 2017-03-21 02:25 PM, Robie Basak wrote:
> 1) "Regression Potential"
> "Regression Potential" is supposed to describe:
> ...how regressions are most likely to manifest, or may manifest
> even if it is unlikely, as a result of this change. It is
> assumed that any SRU candidate patch is well-tested before
> upload and has a low overall risk of regression, but it's
> important to make the effort to think about what could happen in
> the event of a regression.
> Note that "Low" or "None", or an explanation of why it is "Low" or
> "None", is insufficient and doesn't meet this requirement.
> If I don't have enough information to be able to fill this in myself
> quickly, or if I have a particuarly big queue when I'm reviewing, I will
> continue to bounce this back to the uploader and delay the SRU. I prefer
> this over accepting something that will get insufficient verification
> and risk regressing the stable release.
I think that's correct, with one caveat. It's not gaining us anything if
what is added is beyond unlikely. SRUs need to be evaluated on a case by
case basis, sometimes the best you can say is "This has a low potential
of regression because $reasons.", but those are indeed not the norm.

> 2) "verification-done"
> When marking "verification-done", please describe what packages were
> tested and what versions. This is explicitly requested in the acceptance
> message, but I see many people not doing this.
I agree, but this is valid criticism for any change on any bug on
Launchpad. Marking something as Confirmed, or Invalid, or even removing
tasks without a comment explaining why is just bad form.

It's nice to write this out here, but perhaps it would be good to update
the templates for comments on the bugs when a SRU lands in -proposed to
clarify it? I would not expect all our contributors on SRU bugs to see
ubuntu-devel@ discussions.

Mathieu Trudel-Lapierre <[email protected]>
Freenode: cyphermox, Jabber: [email protected]
4096R/65B58DA1 818A D123 0992 275B 23C2 CF89 C67B B4D6 65B5 8DA1