Re[2]: Summary of store_url project and some questions before posting some patches.

>---- Original Message ----
>From: Alex Rousskov <rousskov_at_measurement-factory.com>
>To: "Eliezer Croitoru" <eliezer_at_ngtech.co.il>
>Cc: "Squid Developers" <squid-dev_at_squid-cache.org>
>Sent: Sat, Oct 6, 2012, 11:49 PM
>Subject: Re: Summary of store_url project and some questions before posting some patches.
>
>On 10/06/2012 09:24 AM, Eliezer Croitoru wrote:
>> As I moved forward and managed to make store_url feature stable to pass
>> all of my tests The next step is to state a summary of the feature.
>>
>> The goal of store_url feature is to give squid a way to handle
>> De-duplication of objects.
>>
>> Code implementation is quite simple but since I was working with the old
>> bzr revision 12317 ant now its about 30 revisions up I dont have a clue
>> on what to do.
>> for now I downloaded the new 2a revision 12349.
What do you think about that? does it matters at all?

>>
>>
>> How it was done in squid 2.7 is less relevant for squid 3 branch
>> implementation and was reviewed in the other thread.
>>
>> We discussed earlier on "url" to "originalUrl" refactoring and it seems
>> to me that this will make a big and unneeded modification to the code
>> that is not needed to achieve the goal.
>
>One reason to rename the old "url" field is so that we can double check
>that you caught all usage cases. If you do not rename, the patch will
>not show the cases you missed (if any). Any renaming changes for this
>reason alone can be later dropped (after reviews and before the commit).
>
>Another reason is to alert future developers that they are dealing with
>a URL which may be different from the store URL. If this is a valid/good
>reason, the renaming should stay.
Ok seems reasonable to me.

>
>
>> Also At every place the store_url is mentioned can be the indication of
>> it while every other place a "url" or "canonicalUrl" is mentioned will
>> be the mark of original url usage.
>
>Yes, but those "every other places" will not be visible in the patch and
>since "url" is a rather generic term some of them may be difficult to
>find in Squid sources using a manual search.
>
I wont say that there is no possiblitiy but I didnt used only manual search for it since it's unreasonable in such a huge ammount of code.
I indeed found one place that I needed to change the code and since it was in a #IF statement only the compiler was able to recognize it.

 
>
>HTH,
>
>Alex.
Thanks,
Helps a lot!
Received on Sat Oct 06 2012 - 22:33:28 MDT