yellow team mailing list archive

[Nicola Larosa <nicola.larosa@xxxxxxxxxxxxx>]

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed
File hooks/config-changed (right):

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed#newcode50
hooks/config-changed:50: # Restarting of the gui and api services is
handled below.
gary.poster wrote:
> I think the point of the comment is to reassure readers that these
things
> will be restarted, just not in this code block. "below" means "not in
> this code block."

Oh, I see. I mistook "below" to mean "in the code below, within this
function".

> The comment worked for me.  Would this have been clearer for you?
> "# The juju_gui_source_changed and juju_api_branch_changed variables
> control whether we restart the GUI and the API, respectively, at the
end
> of the function."  Or something.  Other ideas?

Yes, maybe with s/at the end of the function./after this function./, or
similar.

frankban wrote:
> This pre-existed and I think it's actually correct, the services are
not
> restarted here.

Yes, it preexisted, but it was placed *after* a code block, making its
interpretation less ambiguous.

https://codereview.appspot.com/6977043/

-- 
https://code.launchpad.net/~frankban/charms/precise/juju-gui/bug-1088618-serve-releases/+merge/140745
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~frankban/charms/precise/juju-gui/bug-1088618-serve-releases into lp:~juju-gui/charms/precise/juju-gui/trunk.