[Distutils] extends-cache

[Rafael Monnerat]

Thank you for review.

On 14-02-2011 04:54, Thomas Lotze wrote:
> Rafael Monnerat wrote:
>
>> I posted the bugs (I think are 2 diferent) into launchpad with my patches
>> which I made on friday:
> Thank you.
>
>>     - https://bugs.launchpad.net/zc.buildout/+bug/717730
> As far as the cache issue is concerned, this patch looks fine by me, but
> of course, there should be a test for the changed behaviour.
Ok, I will write it and update the patch.

> OTOH, the patch reminds me of a different issue which I've been running
> into with buildout a couple of times now: there should be an API somewhere
> in zc.buildout that helps resolve string values from config files into
> boolean values. I'll remember to bring this up as a separate issue.
After search a little bit I found the API:

"Options.get_bool" on buildout.py

So I think I should use "_dl_options.get_bool('newest')" instead.

>>     - https://bugs.launchpad.net/zc.buildout/+bug/717802
Ok, I agree with your recommendations. I will update the patch soon.
> * A test for the changed behaviour would be in order in this case as well.
>
> * A set might be a more appropriate data structure than a list for
>    collecting the file names seen.
>
> * The empty set of file names should be passed into the first call to
>    _open to avoid a mutable object as the default value of downloaded_list.
>
> * The whole logic of the nested if-else blocks might be refactored to get
>    rid of code duplication (which has been there all along and becomes even
>    more apparent by your patch). Maybe this also makes the appropriate
>    fallback value known at the time of instantiating the download utility.
>