Code review comment for lp:~vorlon/ubuntu/saucy/clock-setup/lp.1184006

Revision history for this message
Colin Watson (cjwatson) wrote :

I'm not sure this really addresses the comments I left in the sysvinit MP:

  We'll need to think about upload coordination. You have a version guard, but what about the overlap case with a daily build where an older installer installs a newer initscripts? That suggests to me that you shouldn't write to /etc/init/hwclock.override or /etc/init/hwclock-save.override if they already contain UTC settings; I think if we changed the installer first to handle either old or new, and then changed sysvinit with that refinement, then that would work.

So how about checking whether /etc/default/rcS contains UTC=.*, and if so assume that it should still be modified rather than hwclock*.override?

I also note that there's no default set for UTC in /etc/init/hwclock*.conf, and the test is [ "$UTC" = yes ]. Won't that mean an unset $UTC implies local time, which is not what's implemented here? Perhaps adding 'env UTC=yes' to /etc/init/hwclock*.conf would be appropriate.

Finally, we actually use lp:~ubuntu-core-dev/clock-setup/ubuntu rather than lp:ubuntu/clock-setup, and they don't share history; could you prepare a new branch based on that? Thanks.

review: Needs Fixing

« Back to merge proposal