SRU shift report: 2020-09-23

[Scott Moser]

On Wed, Sep 23, 2020 at 1:41 PM Robie Basak <robie.basak at ubuntu.com> wrote:
>
> On my SRU shift today, I'm disappointed to report that out of sixteen
> packages I looked at, I didn't end up accepting a single one. Some of
> this is because I ran out of time, but I am writing this report because
> I think that with some help from you, we can do much better.

Thank you for this email.  I do understand that review is difficult, and I
also really appreciate Robie's careful approach to SRU.

I think that some better tools for SRU team members would really help
make things easier.

In looking at the change myself, I think that tools to generate a
patches/applied branch of the SRU upload would be very helpful here.
If you could have done something like:

    $ git-ubuntu add-dsc --name=candidate cloud-utils-0.27-0ubuntu25.2.dsc
    ...
    $ git diff pkg/ubuntu/xenial-devel candidate/unapplied
    ...
    +++ b/debian/patches/series
    @@ -1,3 +1,5 @@
     sync-to-trunk.patch
     lp-1741300-fix-qcow-nbd.patch
     lp-1762748-growpart-grow-past-2TB-disks.patch
    +lp-1493188-support-overlay-filesystem
    +lp-1630274-mount-overlay-first

     $ git show
candidate/unapplied:debian/patches/lp-1493188-support-overlay-filesystem
\
        candidate/unapplied:debian/patches/lp-1630274-mount-overlay-first
| diffstat
     bin/mount-image-callback             |   69 ++++++++-------
     cloud-utils/bin/mount-image-callback |   34 ++++++-
     cloud-utils/test/test-mic            |  157
+++++++++++++++++++++++++++++++++++
     3 files changed, 223 insertions(+), 37 deletions(-)

That is somewhat daunting.  But if you can look at the applied change
its much less so.

    $ git diff pkg/applied/ubuntu/xenial-devel..candidate/applied  |
        filterdiff --exclude "*/debian/*" | diffstat -p1
     bin/mount-image-callback |   59 +++++++++++++----
     test/test-mic            |  157
+++++++++++++++++++++++++++++++++++++++++++++++
     2 files changed, 201 insertions(+), 15 deletions(-)

    $ git diff pkg/applied/ubuntu/xenial-devel..candidate/applied
bin/mount-image-callback | pastebinit -fdiff
    https://paste.ubuntu.com/p/9K4f5YNBXf/

> ## cloud-utils (Xenial)
>
> Substantial diff (non-trivial change) will take considerable time to
> review; postponed for a subsequent pass through the queue.
>
> Outcome: deferred for a second pass over the queue[2].

This change is not as big as it looks. 157 of the lines of diff are an
added test (test/test-mic) runs as part of autopackage tests in newer releases.
For better or worse, xenial will not run autopackage tests (no
debian/tests) so this
is ignorable.

That leaves us with 1 file changed, and when applied, the diff against
pkg/applied/ubuntu/xenial-devel that looks like
https://paste.ubuntu.com/p/9K4f5YNBXf/

Again, I respect the difficulty in reviewing large numbers of
very different source code changes.  But I disagree that this
specific change should qualify as "Substantial diff (non-trivial change)".

I'm willing to remove the test file if that makes this more clear, but
personally I think the ease and usefullness cherry picking upstream
changes into debian/patches justifies the added cost.