Re: [PATCH] remove squid-old.h

On 14/08/2012 10:12 p.m., Kinkie wrote:
>> This branch won't pass a sourcemaintenance.sh run ...
>> * Please run that script to find .h and .cci files which now contain
>> squid.h - they should never include it.
> Done. Expectedly, nothing broke from the removal.
>
>> * Please also check for files like src/ClientDelayConfig.cc and like
>> src/DelayId.cc and src/DelaySpec.cc - which now include squid.h multiple
>> times.
> Fixed.
>
>> src/DescriptorSet.cc:
>> please do not add comments on the same line as the #include. It is NOT
>> accepted by all precompilers. Use a line before the #include.
>> there are mix of C-style and C++-style comments being added in this way by
>> this branch.
> Ok, cleaning up. There's a few of those scattered elsewhere as well,
> I'm fixing those as well.
> Generally speaking, those comments are not really meaningful. Once
> protos.h, typedefs.h and maybe structs.h are split into their
> components, then it will be the right time to review and eliminate.
>
>> src/HttpRequest.h:
>> - you fixed the dependency with pinnedConnection - can the un-ordering
>> shuffle of these #include lines be reverted to the sorted order now?
> It was already done.
>
>> src/auth/Acl.cc:
>> - why is old copyright blurb being added to src/auth/Acl.* ?
> No idea. Removed. But we should define a policy on a standard preface
> to .h and .cc files, if we don't have one.
>
>> src/client_side.cc:
>> - strangely moves the macro define of comm_close() into the system headers
>> list.
> Moved entirely after headers, I'm not sure it's the proper place though.
>
>> src/protos.h:
>> - some of the SQUIDCEXTERN removals are leaving local-scope defines. ie
>> they are missing the "extern" entirely after this patch. Is this good?
> It works, but it's not really intended. Restoring.
>
>> src/store_digest.cc:
>> - store_digest registration is being erased. Why? and if thats okay why not
>> erase the now empty static function too?
> It's not intended. Restoring.
>
>> There is a bit of branch cruft in the patch:
>> * removed file 'include/leakcheck.h'
>> * added file 'include/leakcheck.h'
>> * added file 'src/fs/ufs/RebuildState.cc.moved'
>> * added file 'src/fs/ufs/RebuildState.h.moved'
>> * ... plus several others - search for .moved
> I'll make sure that these are forgotten upon merge. In the meantime,
> I'm trying to do a by-hand patch, attached.
>

+1.

Amos
Received on Tue Aug 14 2012 - 11:26:05 MDT