SRU quality and preventing regressions

[Robie Basak]

Hi Mathieu,

On Tue, Mar 28, 2017 at 02:17:29PM -0400, Mathieu Trudel-Lapierre wrote:
> > "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.

Sure, I have no problem with being told why the regression potential is
low. But the SRU policy currently says:

  * It is assumed that any SRU candidate...has a low overall risk of
    regression...

  * ...it's important to make the effort to think about what could
    happen in the event of a regression.

I'm just implementing the current SRU policy as stated. If the second
point isn't covered, then the provided SRU information doesn't meet SRU
policy.

If there's some exceptional reason why the SRU policy should be bent for
a particular case, I'd be happy to consider that. Appropriate pragmatism
is my understanding of how Ubuntu development works. But in that case,
the exceptional reason needs to be documented in the bug, and it would
help if it were explicit that an exception is being requested.

If the risk is beyond unlikely, a justification as to why that is the
case is fine. But a missing justification is a reject in my book, and
that's the point I wanted to make.

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

The automatic instructions do already say:

    If this package fixes the bug for you, please add a comment to this
    bug, mentioning the version of the package you tested...

and:

    In either case, details of your testing will help us make a better
    decision.

So I think we have already been saying this, but perhaps it could be
made clearer. Personally, I'd like to make it clearer that I want:

  1) Package names tested (where the bug has tasks for multiple
     packages).

  2) Version strings of packages tested (shortcuts like "from
     yakkety-proposed" isn't enough, as the answer to that changes and
     mistakes have been made in the past).

  3) A summary of the testing performed; "as described in Test Case" is
     sufficient if the description there is clear enough.

Robie
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/ubuntu-devel/attachments/20170412/e3b0a02d/attachment.pgp>