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.
_______________________________________________
Distutils-SIG maillist - [email protected]
http://mail.python.org/mailman/listinfo/distutils-sig
