Thanks Alex and Amos,
The Audit gave me a lot!
I hope to sit on it today and if not today later next week.
For now I am using this patch in a testing machine to make sure it does
what needed for a reasonable period of time without bad side effects.
How long it is before committing a patch to trunk?(after all the fixes)
it would need approval of the others and 10 more days?
Thanks,
Eliezer
On 12/6/2012 9:04 AM, Alex Rousskov wrote:
> On 12/05/2012 06:20 PM, Amos Jeffries wrote:
>> - for store_id it is not clear from the name what it is exactly.
>> "Storage of StoreID for the specific cases that the request not avalile"
>> leaves me just as much in the dark about what this variable holds.
>
> Agreed, especially since this is a request member so request is
> naturally available. This sounds like a comment cut-and-paste error of
> some sorts because the patch always sets this data member after a
> successful helper response.
>
>
>> + Prefer something like: "The ID string used internally by Squid to
>> uniquely de-duplicate this requests URL with other URLs stored objects."
>
> I find "uniquely de-duplicate" too puzzling. If you want to briefly
> define what store ID is here, how about this:
>
> If defined, store_id_program mapped requested URL to this ID.
> Store uses this ID (and not the URL) to find and store entries,
> avoiding caching duplicate entries when different URLs point to
> "essentially the same" cachable resource.
>
> If you do not want to define the concept itself here, then the first
> line would suffice IMO.
>
>
> BTW, when you do not think the patch is ready for commit, please use
> [PREVIEW] Subject prefix or no prefix at all. The [PATCH] prefix is for
> patches that you think should be committed essentially without changes.
>
>
> HTH,
>
> Alex.
>
-- Eliezer Croitoru https://www1.ngtech.co.il sip:ngtech_at_sip2sip.info IT consulting for Nonprofit organizations eliezer <at> ngtech.co.ilReceived on Thu Dec 06 2012 - 08:04:08 MST
This archive was generated by hypermail 2.2.0 : Thu Dec 06 2012 - 12:00:10 MST