Re: [PATCH] protos.h splitting, part 2 of 2

On 2/09/2012 6:49 a.m., Kinkie wrote:
> Hi all,
> the attached patch:
> - finishes the job of splitting protos.h into individual components
> - removes the inclusion of protos.h from most files
> - cleans up leftover $Id$ and arch-tag comments
> - cleans up some module initialization calls from #ifdef-d to NOP
> style, as we have discussed on the -dev mailing-list a few days ago.
>
> protos.h is still there, as it contains the prototypes for three
> functions implemented in main.cc. Getting rid of that will require
> some more effort.
>
> It's been build-tested OK on all farm nodes where trunk builds; it may
> mess some things up on windows port but it's currently impossible to
> verify it.
> Could you please review it?
>
> Thanks.
>

include/Range.h:
* including src/Debugs.h from an includes/ file is a dependency problem
waiting to happen. Please move Range.h to src/base/Range.h if the
Debugs.h is actually a required include.

src/HttpHeader.h, src/HttpHeaderTools.cc, src/HttpHeaderTools.h,
src/WinSvc.h, src/peer_proxy_negotiate_auth.h, src/snmp_core.h,
src/store_rebuild.h, src/tests/stub_ipc.cc, src/win32.h:
* adding useless double empty lines.

src/structs.h, src/typedefs.h:
* leaving double empty lines, please remove one of those along with the
list node structure.

src/tests/stub_tools.cc:
* the comment before #incldue tools.h, about tools functions defined in
protos.h is now bogus. please remove.

It would seem your branch merge point is not up to date past the
empty-lines removal that was done on trunk. There area a few chunks
indicating empty line removal when the doubled line does not exist on
current trunk. The login= handling change in src/http.cc as well is also
out of sync. You will need to re-test after these updates.

Amos
Received on Mon Sep 03 2012 - 03:18:59 MDT