Re: [PATCH] remove squid-old.h

From: Kinkie <gkinkie_at_gmail.com>
Date: Tue, 14 Aug 2012 12:12:20 +0200

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

-- 
    /kinkie

Received on Tue Aug 14 2012 - 10:12:30 MDT

This archive was generated by hypermail 2.2.0 : Tue Aug 14 2012 - 12:00:06 MDT