[Merge] lp:~seb128/update-notifier/systemd into lp:update-notifier
Martin Pitt
martin.pitt at ubuntu.com
Wed Jul 20 16:34:47 UTC 2016
Review: Needs Fixing
Looks mostly good, thanks for working on this! I found some nitpicks.
Diff comments:
>
> === added file 'debian/systemd/unicast-local-avahi.path'
> --- debian/systemd/unicast-local-avahi.path 1970-01-01 00:00:00 +0000
> +++ debian/systemd/unicast-local-avahi.path 2016-07-20 16:02:02 +0000
> @@ -0,0 +1,6 @@
> +[Unit]
> +Description=Notification regarding avahi disabled due to .local domain
Nitpick: This should be different than the .service description, easier to tell apart in the logs for debugging. "Path trigger for Avahi .local domain notifications"?
> +PartOf=graphical-session.target
> +
> +[Path]
> +PathChanged=/var/run/avahi-daemon/disabled-for-unicast-local
Just /run please, no new instances of /var/run. For gold stars, change all other occurrences of /var/run to /run in update-notifier. :-)
>
> === added file 'debian/systemd/update-notifier-crash.path'
> --- debian/systemd/update-notifier-crash.path 1970-01-01 00:00:00 +0000
> +++ debian/systemd/update-notifier-crash.path 2016-07-20 16:02:02 +0000
> @@ -0,0 +1,6 @@
> +[Unit]
> +Description=Notification regarding a crash report
Dito, "Path trigger for Apport crash notifications" or so?
> +PartOf=graphical-session.target
> +
> +[Path]
> +PathChanged=/var/crash/
>
> === added file 'debian/systemd/update-notifier-crash.service'
> --- debian/systemd/update-notifier-crash.service 1970-01-01 00:00:00 +0000
> +++ debian/systemd/update-notifier-crash.service 2016-07-20 16:02:02 +0000
> @@ -0,0 +1,6 @@
> +[Unit]
> +Description=Notification regarding a crash report
Add PartOf=graphical-session.target please, this is potentially a long-running service and it should cleanly stop with logging out of the session.
> +
> +[Service]
> +ExecStart=/usr/lib/update-notifier/update-notifier-crash
> +Type=oneshot
>
> === added file 'debian/systemd/update-notifier-release.path'
> --- debian/systemd/update-notifier-release.path 1970-01-01 00:00:00 +0000
> +++ debian/systemd/update-notifier-release.path 2016-07-20 16:02:02 +0000
> @@ -0,0 +1,7 @@
> +[Unit]
> +Description=Notification regarding a new release of Ubuntu
"Path trigger.."
> +PartOf=graphical-session.target
> +
> +[Path]
> +PathChanged=/var/lib/ubuntu-release-upgrader/release-upgrade-available
> +
Trailing empty line
>
> === added file 'debian/systemd/update-notifier-release.service'
> --- debian/systemd/update-notifier-release.service 1970-01-01 00:00:00 +0000
> +++ debian/systemd/update-notifier-release.service 2016-07-20 16:02:02 +0000
> @@ -0,0 +1,6 @@
> +[Unit]
> +Description=Notification regarding a new release of Ubuntu
PartOf=g-s.t
> +
> +[Service]
> +ExecStart=/usr/lib/ubuntu-release-upgrader/check-new-release-gtk
> +Type=oneshot
>
> === modified file 'debian/update-notifier-cds.conf'
> --- debian/update-notifier-cds.conf 2015-01-20 09:34:45 +0000
> +++ debian/update-notifier-cds.conf 2016-07-20 16:02:02 +0000
> @@ -13,7 +13,7 @@
> DATE=$(date)
> echo "$DATE CD found at $DEVNAME"
> sleep 10
> -MOUNTPOINT=$(printf $(awk "{if (\$1 == \"$D\") {print \$2}}" /proc/mounts ))
> +MOUNTPOINT=$(printf $(awk "{if (\$1 == \"$DEVNAME\") {print \$2}}" /proc/mounts ))
*brown paperbag* thanks for cleaning up after me!
> echo "CD mounted at $MOUNTPOINT"
> if [ -L "$MOUNTPOINT/ubuntu" ] || [ -e "$MOUNTPOINT/cdromupgrade" ]
> then
--
https://code.launchpad.net/~seb128/update-notifier/systemd/+merge/300634
Your team Ubuntu Core Development Team is requested to review the proposed merge of lp:~seb128/update-notifier/systemd into lp:update-notifier.
More information about the Ubuntu-reviews
mailing list