Code review comment for ~ahasenack/ubuntu/+source/base-files:groovy-motd-news-config-split

Revision history for this message
Bryce Harrington (bryce) wrote :

I also did a review on the new package motd-news-config. A few notes:

* debian/copyright

This appears to still have template content needing filled out. I don't think the copyright file needs much detail; this is a pretty minimal package.

* debian/motd-news-config.motd-news.default

The grammar of the comments in this file is rather poor. Given that we're making a new package, this seems like a good opportunity to improve the user documentation text. Here's my attempt:

# Enable/disable the dynamic MOTD news service.
#
# This service provides information relevant to the local
# system administrator such as availability of upgrades,
# system status data, and similar.
#
ENABLED=1

# Source service for dynamic MOTD news.
#
# MOTD news is provided by Canonical via motd.ubuntu.com. This
# configuration parameter allows pulling it from one or more
# alternative locations (whitespace-separated). For security
# reasons, these must use the https protocol and have a valid
# certificate.
#
URLS="https://motd.ubuntu.com"

# Delay (in seconds) between MOTD fetch attempts.
#
# Note that news messages are fetched in the background by
# a systemd timer and will not block boot or login.
#
WAIT=5

I'm kind of curious about the WAIT parameter, am I interpreting this correctly that it polls every 5 seconds? That seems far too frequently so I think I'm missing something here.

I also installed base-files from the PPA, and it correctly pulled in the new package and newer ubuntu-server. It appears if you delete (or don't have) 50-motd-news, it'll prompt you to create it but default to omitting it, which seems to be appropriate behavior:

Preparing to unpack .../base-files_11ubuntu11_amd64.deb ...
Warning: Stopping motd-news.service, but it can still be activated by:
  motd-news.timer
Unpacking base-files (11ubuntu11) over (11ubuntu7) ...
Setting up base-files (11ubuntu11) ...

Configuration file '/etc/update-motd.d/50-motd-news'
 ==> Deleted (by you or by a script) since installation.
 ==> Package distributor has shipped an updated version.
   What would you like to do about it ? Your options are:
    Y or I : install the package maintainer's version
    N or O : keep your currently-installed version
      D : show the differences between the versions
      Z : start a shell to examine the situation
 The default action is to keep your current version.
*** 50-motd-news (Y/I/N/O/D/Z) [default=N] ?

review: Approve

« Back to merge proposal