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

Kinkie gkinkie at gmail.com
Mon Aug 31 10:19:00 UTC 2015


>>> 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.
>>>
>>
>> Can't do. IRCB takes an enum as argument; we need to get IRCB out of
>> typedefs to do that. Without going into the AsyncJob rework you were
>> referring to, where could it be shuffled to?
>>
>
> Sorry I was not being clear still. What I meant by "only" at the
> beginning was that enums.h move/remove/add as an example of the limit of
> changes related to enums which might be in scope. If part (ie. removing
> enume.h from typedefs.h) is not possible, then by all means avoid that too.

Ok


>>> 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
>>>
>>
>> Can't find it, where?
>>
>
> Sorry that was in src/ClientInfo.h on the new #include

Ok, done.

>>> In src/SquidTime.h
>>>
>>> * s/miliseconds/milliseconds/
>>>
>>> * s/*Use/* Use/
>>>
>>
>> Ok
>>
>
> lol. Now the space at the end is missing. oops.

Fixed :)

>
>
>
>>>
>>> In src/fs/forward.h
>>>
>>> * missing empty line after the copyright
>>> * double-empty line needs removing
>>> * missing empty line at end of file
>>>
>>
>> Ok for the first two, empty line at end of file seems present to me..
>>
>
> Oh well, the maintenance tools will catch it.

Pre-emptively ran them.

>>> NP: add to our TODO list that fs/rock/RockForward.h should be
>>> fs/rock/forward.h and lacks documentation. Sigh.
>>>
>>
>> Either I do that now, or I suspect it won't be done. Should I?
>>
>
> Next once this is in maybe. Or I could.
>
> Looks okay to me. +1 if it still builds after the audit tweaks.


Ok, merged.

Thanks

-- 
    Francesco


More information about the squid-dev mailing list