Code review comment for ~fnordahl/ubuntu/+source/libvirt:bug/1892132-focal

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

We've just completed the former libvirt SRU - well actually it still is in phasing but it can be considered complete. So it is the right time to do this.

* Changelog:
  - [x] changelog entry correct version and targeted codename
    There is
       libvirt | 6.0.0-0ubuntu8.12 | focal-proposed | source
    you'd need to rebase this

  - [+] changelog entries correct
  - [+] update-maintainer has been run

* New Delta:
  - [-] patches match what was proposed/committed upstream
  - [-] patches correctly included in debian/patches/series
  - [-] patches have correct DEP3 metadata

Both patches are adapted due to not using auto-free back then.
But both LGTM and are properly marked "Origin: backport," - thanks.

It isn't strictly required, but libvirt has that many patches that Ubuntu and started to structure them in directories.
You'll have (a subset of) directories in debian/patches for
- debian (custom patches for Debian)
- stable (pre release stable updates)
- revert (plain reverts of upstream)
- forwarded (a patch created for U/D and forwarded to upstream)
- ubuntu (custom patches for Ubuntu)
- ubuntu-aa (custom patches for Ubuntu in regard to apparmor)

Not all are sorted yet, but at least those we add we try to keep in line, therefore I'd ask to put them into debian/patches/ubuntu if that isn't too much to ask for.

In regard to metadata you have origin - thanks, that probably is the most important one.
There are many that can be added, but of the somewhat required fields you miss out on adding
Bug-Ubuntu: https://bugs.launchpad.net/bugs/<todo-bug>
Last-Update: 20xx-mm-dd

Also there is no pain, but some gain to keep the original patch description in the header.
In this case it explains a lot, no need to remove it here.

* Git/Maintenance
  - [+] testcases added or not needed for this
  - [-] commits are properly split (more important on -dev than on SRUs)

It is not strictly needed for SRUs, but still helps later maintenance (e.g. to cherry pick the changes but not the CL which always is different) if code-changes and changelog would be in individual commits. If you don't mind changing that would be appreciated.

* Build/Test:
  - not checked, is there a PPA with this somewhere already?

review: Needs Fixing

« Back to merge proposal