Code review comment for lp:~veebers/juju-ci-tools/initial-log-forwarding-tests
Revision history for this message
| Martin Packman (gz) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
Have a bunch of random comments inline. Overall looks like a good approach, my main points of feedback:
* Assess scripts this complex can benefit from a bit more structure. A bunch of cooperating functions gets hard to follow around the 200 line mark. An encapsulating class for each environment seems reasonable.
* A bunch of the setup work is done by manual ssh steps on charms. Ideally, most of this complexity is encapsulated in the charm instead.
* I'd generally have more unit tests, but can see how a bunch of the current code is not amenable to it.