Code review comment for lp:~gary/charms/precise/juju-gui/download-cache

Revision history for this message
Francesco Banconi (frankban) wrote :

A very nice improvement Gary, thank you!
Running "make" successfully checked out the download cache branch in my
sandbox/tests/ directory.
LGTM with two comments, one below, one here:

"make deploy" is implemented in tests/deploy.py. As you can see, it
creates a temporary charm repository where the charm is copied using
rsync, and two directories are excluded: .bzr and .venv. I'd be inclined
to also exclude the download cache branch: it's ~16MB and I presume it
would sensibly slows down the deployment process. FWIW, I think it
should work also excluding the whole tests/ directory (it shouldn't be
required in that context), I'll give it a try later.

https://codereview.appspot.com/28770043/diff/1/tests/00-setup
File tests/00-setup (right):

https://codereview.appspot.com/28770043/diff/1/tests/00-setup#newcode33
tests/00-setup:33: if [ ! -f "$ACTIVATE" -o "$TEST_REQUIREMENTS" -nt
"$ACTIVATE" -o "$SERVER_REQUIREMENTS" -nt "$ACTIVATE" ]; then
What do you think about updating the download cache branch when at least
one of the requirement files is outdated?

IIUC, if user X updated the requirements, it is likely that also the
download cache has been updated. In this case, user Y will just run make
to get the new cache branch revision. Subsequently "pip install" can
proceed correctly.

https://codereview.appspot.com/28770043/

« Back to merge proposal