Juju Charms Collection
buildbot-master package

Code review comment for lp:~bac/charms/oneiric/buildbot-master/history-s3

  1. Oneiric (11.10)
  2. history-s3
  3. Merge into trunk
Revision history for this message
Gary Poster (gary) wrote :

Brad, this looks great! Nicely done, and nice to have helpers that can do this for us with future charms.

As I mentioned on IRC, please clean up the log messages in handle_config_changes one way or another, so that we no longer have a colon and two messages when one would do.

Making a chdir context manager would be nice. We have one or two hanging around. You mentioned you had found one in setuplxc.

I asked whether tar xvf will have the expanded file or the existing file win: you said the expanded one, which is good.

You pointed out there are no tests. If there's a sane way to write them then that would be great.

Thank you!

Gary

review: Approve

« Back to merge proposal