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

OK I'll split this out in my merge proposal.

2016-01-28 15:27 GMT+01:00 Pierre-Andre Morey <
<email address hidden>>:

> 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.
>>
>
>