Code review comment for ~ahasenack/ubuntu/+source/cyrus-sasl2:groovy-sasl-disable-doc-build

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
  - [√] changelog entries correct
  - [√] 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
    + But see discussion below
  - [√] autopkgtest against the PPA package passes
    + Skip in lxc; see below
  - [-] sanity checks test fine

Installing sasl2-bin results in some man pages installed to /usr/share/man/man8 and /usr/share/man/man1 that appear to match the bins installed by the package. The man pages display fine and are formatted correctly.

I was also able to install cyrus-sasl2-doc. As expected it's missing a lot of stuff, but the top level framework skeleton is there:

$ dpkg -L cyrus-sasl2-doc
/.
/usr
/usr/share
/usr/share/doc
/usr/share/doc/cyrus-sasl2-doc
/usr/share/doc/cyrus-sasl2-doc/LDAP_SASLAUTHD.gz
/usr/share/doc/cyrus-sasl2-doc/NEWS.Debian.gz
/usr/share/doc/cyrus-sasl2-doc/changelog.Debian.gz
/usr/share/doc/cyrus-sasl2-doc/copyright
/usr/share/doc/cyrus-sasl2-doc/developer.html
/usr/share/doc/cyrus-sasl2-doc/download.html
/usr/share/doc/cyrus-sasl2-doc/genindex.html
/usr/share/doc/cyrus-sasl2-doc/getsasl.html
/usr/share/doc/cyrus-sasl2-doc/index.html
/usr/share/doc/cyrus-sasl2-doc/operations.html
/usr/share/doc/cyrus-sasl2-doc/packager.html
/usr/share/doc/cyrus-sasl2-doc/search.html
/usr/share/doc/cyrus-sasl2-doc/setup.html
/usr/share/doc/cyrus-sasl2-doc/support.html
/usr/share/doc-base
/usr/share/doc-base/cyrus-sasl2-doc

Of course, the links in these html files are largely broken. What I'm wondering is if it might be better to make the cyrus-sasl2-doc binary package uninstallable?

I also ran the autopkgtests but they did not seem to do anything interesting in my lxc container. At least, it seems not to care about the missing docs:

autopkgtest [01:08:35]: testing package cyrus-sasl2 version 2.1.27+dfsg-2ubuntu1~ppa1
autopkgtest [01:08:35]: build not needed
* SKIP no tests in this package
autopkgtest [01:08:35]: @@@@@@@@@@@@@@@@@@@@ summary
* SKIP no tests in this package

Anyway, apart from the question on the docs package installability everything else looks fine to go in.

review: Approve

« Back to merge proposal