[squid-dev] [PATCH] splitting typedefs.h and enums.h

[Amos Jeffries]

On 30/08/2015 2:20 p.m., Kinkie wrote:
> Hi,
>   this is a snapshot of my current effort to split typedefs.h and enums.h,
> removing unneeded definitions and shuffling the remaining ones to locations
> where they are actually used.
> 
> If there are no objections, I would consider this to be a merge candidate,
> although the effort is not yet completed. typedefs.h is almost complete,
> and enums.h just started. I'd like to carry on with the latter, but it's
> already a big patch as it is.
> 
> Thanks
> 

grumblie: I asked you to avoid touching enums during the typedefs.h scope.

What I would like to see in this patch scope is only removing the
#include enums.h from typedefs.h and adding *only* equivalent #include
enums.h directly into other files which break as a result. No shuffling
of enums content themselves.

All of this shuffling risks build failures and bitrot in components we
might overlook. Which means backportage when its discovered.

The bigger the patch is the more difficulty to backporting efforts when
only a small piece of it needs to accompany a portage patch. The
typedefs change by the nature of typedef affects far fewer portages than
enums will. Theoretically each of these symbol changes should be in a
different commit, but that is taking it a little far in the opposite
direction for typedefs. (I will be requesting much lower granularity for
enums however.)

NP: yes these are mostly the same arguments we hear from Alex about HERE
removal. And for good reason. They are valid in both discussions.




In src/CacheDigest.h

* you can remove the "for cache_key" comments now. Its obvious enough
from the filename.
 - the comment was only there to see why typedefs.h was still needed.
 - same in src/tests/stub_CacheDigest.cc

* kb_t comment is outdated now


In src/DiskIO/DiskThreads/DiskThreadsDiskFile.h:

* please take the opportunity to add the empty line above the #include list.


In src/SquidTime.h

* s/miliseconds/milliseconds/

* s/*Use/* Use/


In src/comm/Connection.h

* please try to keep the #include sort order alphabetical, ignore the
#if/#endif protection when considering that manually.
 - ip/* goes after eui/*
 - similar in src/ipc/MemMap.h, src/ipc/StoreMap.h,


In src/comm/forward.h

* please take the opportunity to finally document what "PF" is:
  /// legacy CBDATA callback functions ABI definition for read or write
I/O events
  /// \deprecated use CommCalls API instead where possible


In src/fs/forward.h

* missing empty line after the copyright
* double-empty line needs removing
* missing empty line at end of file

* please take the opportunity to add documentation of what the types
shuffled into there are.
 - probably a good idea to dig into the old wiki ProgrammingGuide or
store code to find out if anyone has already written a good description
documenting them somewhere else.


In src/fs/rock/RockRebuild.cc

* check if fs/rock/RockForward.h is already being included by the other
rock/*.h files indirectly.
 - If so fs/foward.h is unnecessary.
 - if not then it should be #include'd instead of fs/forward.h

NP: add to our TODO list that fs/rock/RockForward.h should be
fs/rock/forward.h and lacks documentation. Sigh.


In src/mem/forward.h

* please mark FREE with a "/// \deprecated use MEMRPROXY_CLASS instead"



Amos