Code review comment for ~athos-ribeiro/ubuntu/+source/apache2:postinst-triggered

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Thanks, Sergio.

I addressed the cosmetic comments and uploaded.

Uploaded:

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading apache2_2.4.58-1ubuntu6.dsc: done.
  Uploading apache2_2.4.58-1ubuntu6.debian.tar.xz: done.
  Uploading apache2_2.4.58-1ubuntu6_source.buildinfo: done.
  Uploading apache2_2.4.58-1ubuntu6_source.changes: done.
Successfully uploaded packages.

The indentation nitpick is actually an issue with that script where some of the "blanks" after the "#" characters are spaces and some are tabs. Since there is no point in carrying a delta to have that fixed, I just replaced the space for a tab there (although other items in the same list will have a space instead).

I also updated all the SRU branches with the same changes.

> For example, when you use "a2enmod" to enable an Apache module, the command doesn't restart apache2 automatically for you; instead, it prints a message saying that you should do it. So I think I'd expect a similar behaviour when it comes to PHP modules...?

This is an interesting thought. I need to assess what other modules are doing. I see most users of apache2_reload are just reloading instead of restarting, but there may be other modules that are just not using the maintscript helper, so I'd need to check those as well.

FWIW, the change should enable reloading (or doing other actions) through the script as well. Currently, it mostly tell the script to skip/noop (if used properly) when called from a trigger.

I will add an entry to the PHP release notes for this. I agree this is important enough to be mentioned there.

« Back to merge proposal