Code review comment for lp:~julian-edwards/maas/enlist-nodegroup-bug-1274926

Thanks for reviewing.

On Tuesday 11 Feb 2014 07:00:06 you wrote:
> Review: Approve
>
> Sensible change. I assume autodetection itself is already tested, in a test
> that needed no change. One thing that's a bit awkward (and I do see the
> XXX about it) is that the semantics of the autodetect_nodegroup parameter
> are only explained in the test. I think the representation of booleans is
> not very highly standardised, so it may be worth a note in the docstring
> even if the docstring does not cover all parameters.

I considered it but it felt daft to add just that one param and none of the
others, which is why I XXXed it for later! I'll stick it in the comment for
later inclusion in a decent docstring.

> For the error message in api.py, saying that the nodegroup parameter is
> missing may be too strong. There's a choice to be made. How about "one of
> nodegroup or autodetect_nodegroup must be specified"?

The code doesn't enforce that though, it just enforces the former without the
latter.

I'll land this now and we can discuss a different message later, I gotta run.