Code review comment for lp://staging/~gary/zc.buildout/support-system-python

Revision history for this message
Gary Poster (gary) wrote :

So, I have a, um, 1500+ line additional diff. This further breaks our review policies, which might mean we should think further about how to do these upstream-project reviews in the future. But meanwhile...

https://pastebin.canonical.com/20796/

It was developed to prepare some previous bugfixes for upstream review by adding tests, and to address and give diagnostic tools for bug 407408 ( https://bugs.launchpad.net/launchpad-foundations/+bug/407408 ).

This diff does the following:

- Add new option and tests: ``allowed-eggs-from-site-packages`` allows you
  to specify a glob-aware whitelist of project names that may come from
  site-packages. This defaults to '*', meaning there is no filtering.
- Add feature and test to let you see what eggs have been selected from
  site-packages: "Egg from site-packages: ..." in the output of
  ``bin/buildout -v``.
- Add new option and tests: ``include-site-packages-for-buildout`` allows
  you to control if site-packages are used for the bin/buildout script.
  This defaults to "false," which is a change in behavior! (Rationale is in
  docs.)
- Standardize on one of the several competing approaches for default options
  in the [buildout] section. This removes my previous use of setdefault,
  discussed with flacoste.
- Make generated script paths prettier by eliminating duplicate paths.
- Add comment about recipes using buildout options, not buildout attributes.
  Practice what I preach in zc.recipe.egg.
- Standardize on one approach for handling bool options in zc.recipe.egg. Not
  sure I like it but it is preexisting, and now it is consistent, and it
  behaves well in terms of letting buildout be configured before it looks too
  hard at it.
- Normalize tests.py function docstrings to begin at col 0
- Correct some typos

« Back to merge proposal