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

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:
  - [+] changelog entry correct version and targeted codename
  - [+] changelog entries correct
  - [+] bug references correct

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

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

* 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?

P.S. no need to change this but the branch being called "hiruste" made me fail quite some commands :-)

review: Needs Fixing

« Back to merge proposal