Code review comment for lp://staging/~gary/juju-gui/bug1218924

On 2013/10/14 09:31:41, frankban wrote:
> This branch is very nice Gary, thank you!
> LGTM and QA ok (both local nginx and the deployed charm with sandbox
mode on and
> off). I only have a consideration about isJujucharm, please read
below.

> https://codereview.appspot.com/14516054/diff/1/docs/process.rst
> File docs/process.rst (right):

https://codereview.appspot.com/14516054/diff/1/docs/process.rst#newcode172
> docs/process.rst:172: simulateEvents: false, isJujucharms: true, and
> showGetJujuButton: true.
> After setting isJujucharm to false the GUI seems broken: the charm
browser is
> replaced by a "Hello, world!" and therefore cannot be open.
> This is not a bug of this branch, this happens also when using "make
debug" with
> isJujucharm=true. Setting it to false or navigating, e.g. to
"/sidebar" fixes
> the problem. I'd suggest to leave isJujucharms untouched in this QA
> instructions.

> Minor minor: simulateEvents is already set to false.

Thank you very much for the review and qa, Francesco. I removed the
instructions referencing the isJujucharm flag and filed
https://bugs.launchpad.net/juju-gui/+bug/1239671 to remove the flag
itself eventually. I decided to keep mentioning the simulateEvents
because I wanted people to verify that they were testing with the
simulation turned off.

Gary

https://codereview.appspot.com/14516054/