Implemented the remove_unit functionality

Implemented the remove_unit functionality

In the fakebackend and the PyJujuAPI. It follows the improv
as close as possible in the fakebackend but then only exposes
the same information through the environment as the improv
version did.

https://code.launchpad.net/~hatch/juju-gui/remove-units/+merge/156705

(do not edit description out of merge proposal)

[jeff.pihach, 2013-04-02 21:56:47]

Please take a look.

[matthew.scott, 2013-04-02 22:19:52]

LGTM - thanks for the work on this.

[benji, 2013-04-02 22:22:14]

LGTM, with a couple of small suggestions.

https://codereview.appspot.com/8280044/diff/1/app/store/env/fakebackend.js
File app/store/env/fakebackend.js (right):

https://codereview.appspot.com/8280044/diff/1/app/store/env/fakebackend.js#ne...
app/store/env/fakebackend.js:451: warning: warning
It seems to me that these would make more sense as plurals.

https://codereview.appspot.com/8280044/diff/1/app/store/env/sandbox.js
File app/store/env/sandbox.js (right):

https://codereview.appspot.com/8280044/diff/1/app/store/env/sandbox.js#newcod...
app/store/env/sandbox.js:361: @param {Object} data contains service_name and
num_units required for
Since this has to have a period (per the linter) shouldn't it also start with a
capital? ("Contains")

https://codereview.appspot.com/8280044/diff/1/app/store/env/sandbox.js#newcod...
app/store/env/sandbox.js:381: @param {Object} data contains unit_names to remove
and a calback.
Capitalization, as above.

https://codereview.appspot.com/8280044/diff/1/app/store/env/sandbox.js#newcod...
app/store/env/sandbox.js:391: 
Small suggestion: if it is OK for data.err to be an empty list when there are no
errors, then lines 385-390 could be reduced to.

data.err = res.error
data.result = (res.error.length == 0)

[jeff.pihach, 2013-04-02 22:26:48]

Thanks all for the reviews! Appreciate it.

https://codereview.appspot.com/8280044/diff/1/app/store/env/fakebackend.js
File app/store/env/fakebackend.js (right):

https://codereview.appspot.com/8280044/diff/1/app/store/env/fakebackend.js#ne...
app/store/env/fakebackend.js:451: warning: warning
On 2013/04/02 22:22:14, benji wrote:
> It seems to me that these would make more sense as plurals.
I was following the conventions followed previously.

https://codereview.appspot.com/8280044/diff/1/app/store/env/sandbox.js
File app/store/env/sandbox.js (right):

https://codereview.appspot.com/8280044/diff/1/app/store/env/sandbox.js#newcod...
app/store/env/sandbox.js:361: @param {Object} data contains service_name and
num_units required for
On 2013/04/02 22:22:14, benji wrote:
> Since this has to have a period (per the linter) shouldn't it also start with
a
> capital? ("Contains")

Done.

https://codereview.appspot.com/8280044/diff/1/app/store/env/sandbox.js#newcod...
app/store/env/sandbox.js:381: @param {Object} data contains unit_names to remove
and a calback.
On 2013/04/02 22:22:14, benji wrote:
> Capitalization, as above.

Done.

https://codereview.appspot.com/8280044/diff/1/app/store/env/sandbox.js#newcod...
app/store/env/sandbox.js:391: 
On 2013/04/02 22:22:14, benji wrote:
> Small suggestion: if it is OK for data.err to be an empty list when there are
no
> errors, then lines 385-390 could be reduced to.
> 
> data.err = res.error
> data.result = (res.error.length == 0)
In this case that won't work because the implementations already check for
if(data.err) so it would always be truthy then.

[jeff.pihach, 2013-04-02 22:30:59]

*** Submitted:

Implemented the remove_unit functionality

In the fakebackend and the PyJujuAPI. It follows the improv
as close as possible in the fakebackend but then only exposes
the same information through the environment as the improv
version did.

R=matthew.scott, benji
CC=
https://codereview.appspot.com/8280044