Code review comment for ~ahasenack/ubuntu/+source/base-files:groovy-motd-news-config-split

Revision history for this message
Bryce Harrington (bryce) wrote :

* Changelog:
  - [√] old content and logical tag match as expected
  - [ ] changelog entry correct version and targeted codename
  - [√] update-maintainer has been run

* Actual changes:
  - [-] no upstream changes to consider
  - [-] no further upstream version to consider
  - [√] debian changes look safe

* Old Delta:
  - [-] dropped changes are ok to be dropped
  - [-] nothing else to drop
  - [?] changes forwarded upstream/debian (if appropriate)

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

* Build/Test:
  - [√] build is ok
  - [-] verified PPA package installs/uninstalls
  - [-] autopkgtest against the PPA package passes
  - [√] sanity checks test fine

The changelog entry wording sounds a little mysterious, can I suggest a small wording improvement?

Is:
  "The /etc/default/motd-news conffile is in another src package"

S/b:
  "Split the /etc/default/motd-news conffile to a separate src package, motd-news-config. Force the system to upgrade to ubuntu-server 1.453 at least when this is installed to ensure the conffile remains present."

I know Debian doesn't use our motd-news stuff, but are there any bits of this that might be applicable to Debian? I'm assuming not.

Your testing process looks sound, I didn't run those in detail however I verified the branch builds, and that attempting to install the built deb correctly refuses to do so without the updated ubuntu-server:

triage-groovy+20.10:~/pkg/BaseFiles$ sudo dpkg -i base-files_11ubuntu11_amd64.deb
dpkg: regarding base-files_11ubuntu11_amd64.deb containing base-files:
 base-files breaks ubuntu-server (<< 1.453)
  ubuntu-server (version 1.451) is present and installed.

dpkg: error processing archive base-files_11ubuntu11_amd64.deb (--install):
 installing base-files would break ubuntu-server, and
 deconfiguration is not permitted (--auto-deconfigure might help)
Errors were encountered while processing:
 base-files_11ubuntu11_amd64.deb

Other than the changelog entry everything LGTM, so I'm marking this approved and you can adjust the changelog before landing.

review: Approve

« Back to merge proposal