[Distutils] extends-cache

Thomas Lotze thomas at thomas-lotze.de
Mon Feb 14 07:54:30 CET 2011


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.

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.

>    - https://bugs.launchpad.net/zc.buildout/+bug/717802

* 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.

-- 
Thomas





More information about the Distutils-SIG mailing list