Code review comment for ~bryce/ubuntu/+source/apache2:apache2-fix-lp1832182-hirsute

Lucas Kanashiro (lucaskanashiro) wrote :

The package changes and the SRU bug description LGMT in general, just some comments below:

- In the changelog your target is groovy but this MP is targeting hirsute.

- This is me being nitpick: I would change this commit message:

commit a4a20160eefd3cd0a3cb07d2a95fb9665c1f2b16
Author: Bryce Harrington <email address hidden>
Date: Mon Oct 5 16:03:00 2020 -0700

    * d/apache2ctl: Also use systemd for graceful if it is in use. This extends an earlier fix for the start command to behave similarly for restart / graceful. Fixes service failures on unattended upgrade. (LP: #1832182)

to something like:

    * d/apache2ctl: Also use systemd for graceful if it is in use (LP: #1832182).

    This extends an earlier fix for the start command to behave similarly for
    restart / graceful. Fixes service failures on unattended upgrade.

It is a better reading IMO.

- I see this same bug was reported to Debian here:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=927302

Could we forward these changes to Debian as well?

Apart from that the package built fine and I was able to confirm that the fix is working as expected (following the instructions in the SRU bug). I also confirmed that autopkgtest did not identify any regression (despite it does not exercise the path of proposed changes):

autopkgtest [10:51:10]: @@@@@@@@@@@@@@@@@@@@ summary
run-test-suite PASS
duplicate-module-load PASS
htcacheclean PASS
default-mods PASS
ssl-passphrase PASS
check-http2 PASS
chroot PASS

review: Needs Fixing

« Back to merge proposal