[hipl-dev] Re: [hipl-commit] [trunk] Rev 4479: Removed unnecessary cruft, including:
- From: Diego Biurrun <diego@xxxxxxxxxx>
- To: hipl-dev@xxxxxxxxxxxxx
- Date: Mon, 17 May 2010 12:01:47 +0200
On Tue, May 11, 2010 at 03:17:55PM +0300, Miika Komu wrote:
> On 05/11/2010 11:35 AM, Diego Biurrun wrote:
>
>> On Tue, May 11, 2010 at 10:02:18AM +0300, Miika Komu wrote:
>>>
>>> Log:
>>> Removed unnecessary cruft, including:
>>> * old wireshark patches (hip patch is already in vanilla wireshark)
>>> * test thingies (manual says now to use nc6 instead of conntest stuff)
>>> * old openwrt stuff
>>> * old dht stuff
>>>
>>> Renamed and relocated files to be more consistent:
>>> * binary packaging start up scripts
>>> * firewall config file
>>>
>>> The changes depended on .bzrignore, Makefiles, manual, HACKING and
>>> spec files, so I followed the trail of changes also there.
>>>
>>> Tested:
>>> * make all
>>> * make deb
>>> * make dist
>>>
>>> Note: there is no "test" binary package anymore because it was empty!
>>
>> This commit does a trillion different things all lumped into one and
>> should have been split properly. As is to be expected from such a
>> monster, it was not done correctly.
>
> while I usually slice vertically, this one was horizontally. The commit
> was a tradeoff, I did the best I could with the time limit for today and
> I did not want to delay this further to create further conflicts for me
> or other people. I am sorry if I failed to meet your quality standards.
Your motivation is noble and the end result you aim for is good, but my
point is that you failed to achieve your own objectives: you have not
saved anybody time. Not me, not yourself, not anybody else.
Conflicts are easy to resolve. I don't remember any conflict that took
longer than 10 minutes to fix and that's an absurd outlier, usually it's
in the ballpark of 1-2 minutes.
Contrast that with the handful of hours that you and I invested into
reviewing and fixing this commit...
Whoever tells you that conflicts are more annoying than bugs just does
not care about bugs and/or considers them somebody else's problem.
Since you are the HIPL maintainer, that somebody else is you at the end
of the day. If you take advice from the conflicts-hurt crowd, you are
putting more work on your own plate.
So I think we have a misunderstanding here: I'm not advocating doing
more work, on the contrary, I'm preaching a disciplined working style
that saves time in everything but the very short term...
>> More things, but probably not an exhaustive list:
>>
>>> --- patches/openwrt/package/Makefile 2010-05-03 16:43:45 +0000
>>> +++ packaging/openwrt/package/Makefile 2010-05-11 07:02:11 +0000
>>> @@ -139,14 +127,6 @@
>>>
>>> -define Package/hipl-test/install
>>> - $(INSTALL_DIR) $(1)/etc/hip/test/
>>> -
>>> - $(INSTALL_BIN) ./files/test/* $(1)/etc/hip/test/
>>
>> Something is now amiss with the openwrt package. The files/test
>> directory is no longer referenced anywhere, but it was not removed.
>> I'm not sure it should be removed, but the current situation is
>> clearly wrong.
>
> yes, I noticed this too but I am not sure what to do about this. The
> binaries don't include all test directory stuff, which is fine at least
> for me.
Hmmm, who is currently the local expert for Openwrt builds? Mircea?
> Could somebody try if the OpenWRT port still builds ok? I am terribly
> sorry if I broke it.
It was not building, just look at the mails from the autobuilder.
You moved some stuff around without updating the references.
>> I must say I'm getting weary of reviewing stuff here only to see the
>> same mistakes getting repeated - without fail - at every opportunity
>> that presents itself. This is extremely frustrating.
>
> I grep before commits, but some things just slip through my fingers,
> sorry.
I think it's very hard to do such a large commit correctly. One is
just bound to overlook something. The only sane solution is to split
it into manageable pieces where one can be confident of having done
them correctly and the correctness can be verified - again with
confidence - in a reasonably short time interval...
> Would it be possible to write a separate checker rule to Makefile.am
> that would grep the diffs and prints warnings on lost/suspicous
> things?
That sounds rather tricky to implement correctly. I'd rather see us
become more disciplined...
> I also forgot what was the super clean spell we used last time?
I just use 'grep -ri foo *'.
more things I noticed:
>> Just try grepping the tree for the remainders of "conntest".
test/conntest.h is now a leftover, the .c files were removed without
removing the .h file.
>> The patches/opendht directory is now empty, but still referenced.
Removal still pending. Never mind, I did it.
Diego
Other related posts:
