Code review comment for ~kick-d/ubuntu/+source/ntp:logical/4.2.6.p5+dfsg-3ubuntu9

Thanks, and sorry for this one. I kept this in my head, and forgot to
squash.

2016-01-28 14:31 GMT+01:00 Robie Basak <email address hidden>:

> Hi Pierre,
>
> Good job on this one. The only diff between this and
> 1:4.2.6.p5+dfsg-3ubuntu9 are debian/changelog and update-maintainer as I
> expect.
>
> Good job also squashing down all the different Apparmor-related changes
> into one commit.
>
> My only complaint is on commits 93ec5cb and 8c7e386. These both touch the
> same lines in debian/ntpdate.if-up. In 8c7e386 the "LOCKFILE" line is
> changed and "invoke-rc.d" lines added, and in 93ec5cb the "LOCKFILE" line
> is changed back again and one of the previously added "invoke-rc.d" lines
> is moved. This is churn that we want to squash out at this logical stage.
>
> For example:
>
> @@ -30,7 +30,7 @@ if [ -r /lib/udev/hotplug.functions ]; then
> wait_for_file /usr/sbin/ntpdate-debian
> fi
>
> -LOCKFILE=/var/lock/ntpdate
> +LOCKFILE=/var/lock/ntpdate-ifup
>
> # Avoid running more than one at a time
> if [ -x /usr/bin/lockfile-create ]; then
>
>
> and then later:
>
> @@ -30,7 +30,7 @@ if [ -r /lib/udev/hotplug.functions ]; then
> wait_for_file /usr/sbin/ntpdate-debian
> fi
>
> -LOCKFILE=/var/lock/ntpdate-ifup
> +LOCKFILE=/var/lock/ntpdate
>
> # Avoid running more than one at a time
> if [ -x /usr/bin/lockfile-create ]; then
>
> Indicate churn to me. When figuring out the logical set of changes made,
> these hunks cancel out and so should not appear at all. The same applies to
> one of the two invoke-rc.d lines.
>
> I would split "debian/ntpdate.if-up: Fix interaction with openntpd", which
> I take to be the "if [ -e /usr/sbin/openntpd ]; then" stanza into its own
> commit. Then there would be a separate commit that doesn't need to touch
> LOCKFILE and just adds the two invoke-rc.d lines directly into their final
> places. Since this distils the previous uploads, the commit messages will
> probably need tweaking a little so that they make sense.
>
> Apart from this, everything else is byte-perfect. I'm not sure it's
> necessary to redo this. It's minor enough that I can probably just keep it
> in mind while reviewing your merge. So I'll do that next. No action needed
> here for now. Thanks!
> --
>
> https://code.launchpad.net/~kick-d/ubuntu/+source/ntp/+git/ntp/+merge/284274
> You are the owner of
> ~kick-d/ubuntu/+source/ntp:logical/4.2.6.p5+dfsg-3ubuntu9.
>