yellow team mailing list archive
-
yellow team
-
Mailing list archive
-
Message #01870
Re: [Merge] lp:~bac/charms/precise/juju-gui/1086790 into lp:~juju-gui/charms/precise/juju-gui/trunk
Review: Approve code
The branch looks good. I only had small suggestions or questions.
The "#-*- python -*-" on line 28 surprised me. I am not against that
style of file type markers, but I just wanted to be sure it was
intentional. Oh, and it would be nice if there were a space after the
"#" character. :)
The import on line 130 could be a single-line import.
Line 164: hmm, prefixing the string with a newline will mean that the
log file starts with a blank line. That's not the end of the world, but
it is a bit odd. Maybe a comment to explain why would be in order.
Line 171 of the diff, the contents of the revision file: I /think/ I
saw a recent branch that bumped the revision higher than 13. You might
want to double-check what number the trunk is on.
Line 206: "The monkey patch is undone at the end of the test." Should
that read "The monkey patch is undone in the tearDown method."?
--
https://code.launchpad.net/~bac/charms/precise/juju-gui/1086790/+merge/138823
Your team Juju GUI Hackers is subscribed to branch lp:~juju-gui/charms/precise/juju-gui/trunk.
References