← Back to team overview

yellow team mailing list archive

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