Code review comment for ~ahasenack/ubuntu/+source/zoneminder:focal-zoneminder-mysql8

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

I don't think we need to add the test for it, would it be nice: yes; is it really necessary: no.
At least not needed right now.

The mysql8 patches are a bit messy, but I agree that it does the job in as-minimal-as-possible fashion. And when moving to the next version it can be dropped.

Changelog:
- [√] changelog entry correct version and targeted codename
- [√] changelog entries correct
- [√] update-maintainer has been run

Actual changes:
- [√] no further upstream version that we want right now

New Delta:
- [√] new patches are good and while not matching the final way they are from upstream and do the job for now.
- [√] new patches correctly included in debian/patches/series
- [√] new patches have correct DEP3 metadata linking to discussions and such

Build/Test:
- [√] build is ok
- [√] sanity checks test fine

I tested it with mythzoneminder and the basics there worked - if we could long term add an autopkgtest that would be good (as discussed above).

I ran this with a mix of php 7.4 and zoneminder to represent how focal will soon look like:
... mysql-server-8.0 mysql-server-core-8.0 ocl-icd-libopencl1 php-apcu php-apcu-bc php-gd php-mysql php7.3-cli php7.3-common php7.3-json php7.3-opcache
  php7.3-phpdbg php7.3-readline php7.4-gd php7.4-mysql zoneminder zoneminder-doc ...
All the perl installs also looked a lot.
But so far things worked.

One thing you might want to look at before an upload is this (up to you):
Setting up zoneminder (1.32.3-2ubuntu1~ppa2) ...
I: Generating /etc/zm/core.php from /usr/share/zoneminder/www/api/app/Config/core.php.default ...
zoneminder.conf:1: Line references path below legacy directory /var/run/, updating /var/run/zm → /run/zm; please update the tmpfiles.d/ drop-in file accordingly.

It feels a bit less stable than usual, but ok still: +1 for this

review: Approve

« Back to merge proposal