Code review comment for lp:~gary/zc.buildout/python-support-5-initial-egg-control

Hi Gary,

My only comment is about some API incompatible changes you made. Not sure it's a problem or not. Otherwise, all fine.

> === modified file 'src/zc/buildout/easy_install.py'

> def use_dependency_links(setting=None):
> old = Installer._use_dependency_links
> if setting is not None:
> @@ -858,9 +1014,13 @@
> links=(), index=None,
> executable=sys.executable, always_unzip=None,
> path=None, working_set=None, newest=True, versions=None,
> - use_dependency_links=None, allow_hosts=('*',)):
> + use_dependency_links=None, include_site_packages=None,
> + allowed_eggs_from_site_packages=None, allow_hosts=('*',)):
> installer = Installer(dest, links, index, executable, always_unzip, path,
> newest, versions, use_dependency_links,
> + include_site_packages=include_site_packages,
> + allowed_eggs_from_site_packages=
> + allowed_eggs_from_site_packages,
> allow_hosts=allow_hosts)
> return installer.install(specs, working_set)
>
> @@ -868,9 +1028,14 @@
> def build(spec, dest, build_ext,
> links=(), index=None,
> executable=sys.executable,
> - path=None, newest=True, versions=None, allow_hosts=('*',)):
> + path=None, newest=True, versions=None, include_site_packages=None,
> + allowed_eggs_from_site_packages=None, allow_hosts=('*',)):
> installer = Installer(dest, links, index, executable, True, path, newest,
> - versions, allow_hosts=allow_hosts)
> + versions,
> + include_site_packages=include_site_packages,
> + allowed_eggs_from_site_packages=
> + allowed_eggs_from_site_packages,
> + allow_hosts=allow_hosts)
> return installer.build(spec, build_ext)
>

Should we care about API compatibility in these two functions? You added
parameters before allow_hosts. Theorically, this breaks API compatibility for
call sites passing things by parameters. Arguably a bad idea with such a
signature, but it's your call :-)