Code review comment for ~bryce/ubuntu/+source/php7.4:merge-v7o4o5-1-groovy

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

v7o4o5 is quite the leetspeek try :-) - what is vtoaos? :-P

Changelog:
- [✓] old content and logical tag match as expected
- [✓] changelog entry correct version and targeted codename
- [x] changelog entries not correct
      Line 23,24 and 29 have invalid indent
- [✓] update-maintainer has been run

Actual changes:
- [✓] no major upstream changes to consider having impact on packaging
- [x] hmm further upstream version to consider
    I know we rebase from Debian, but doing 7.4.5 now while 7.4.8 is available.
    Quite some fixes - how about doing this once for groovy ang going to the July version right
    away instead of the April release?
    7.4.8 seems to be a required CVE anyway
    https://www.php.net/ChangeLog-7.php#7.4.8
    https://www.php.net/releases/7_4_8.php
    If you go ahead of Debian for this or ping Ondřej if he'd mind to bump Debian is up to you.
- [✓] debian changes look safe (only a few in 7.4.4-1)

Old Delta:
- [✓] dropped changes are ok to be dropped
- [✓] nothing else to drop
    Well if we'd go to >=7.4.6 another CVE fix would be part of upstreams code.

    The conflicts for upgrades from former versions have to go through Focal.
    Therefore the conflict with 7.2/7.3 will already have happened.
    This surely covers:
  4 - d/control, d/control.in: Conflict with mod-php from php7.2 and
  5 php7.3 to ensure safe upgrade path for apache2.
  6 (Fixes LP #1850933)
    Maybe even this:
  7 - libapache2-mod-php.postinst.extra: Disable other mod-php versions.
  8 Fixes failure when upgrading from previous versions of mod-php.
  9 (LP 1865218)
    IMHO the related Delta can be dropped now (similar delta will come back
    once moving to 7.5). What do you think?

Bonus - if you agree and go to >=7.4.6 there will be NO Ubuntu Delta left.
So it could be a sync (for a short while).

New Delta:
- [✓] no new patches added

Build/Test:
- [✓] build is ok (I agree the few errors don't warrant extra Delta)
- [✓] verified PPA package installs/uninstalls
   (x86 only build , boo - but it seems I'm generally the only one complaining :-) )
- [✓] I've not seen autopkgtest data, but that will happen before migration so we'd catch it

The fixes in changelog are required but not too impactful, the decisions on Delta and Versions are up to you mostly. Since I'm probably away once you resolved these feel free to ask another reviewer or go on with an upload once you addressed my concerns.

review: Needs Fixing

« Back to merge proposal