Code review comment for lp:~tycho-s/maas/add-hardcode-amt-option

On Wed, Apr 16, 2014 at 12:48:40PM -0000, Raphaël Badin wrote:
> Review: Needs Information
>
> Looks good, but please confirm that you've QAed this. This close to the release, we're being cautious for things that touch that part of the code (see [0]).
>
> [0]
>
> I'm assuming this has been tested in the field… right?

Yep. However, it is worth noting that the power_address field doesn't
have any validation (that it is an actual IP address), so if you type
something wrong in there it won't work. This is a bug with every other
IP address field as well, though.

> (Note that the existing unit tests will only check that the template can be rendered correctly)
>
>
> [1]
>
> 31 + 'power_address', "Optional IP to use instead of MAC"),
>
> This is a bit vague (what does it mean to use an IP instead of a MAC). *I* understand what you mean because I know what this is about but you have to put yourself in the shoes of a user. Maybe have a look at the comments for IPMI parameters for inspiration.

Done.

> [2]
>
> Please confirm this with Andres but I think this needs to be linked to a bug in order to make it into 1.5.

Checked with him, we do, and I've created one and linked it everywhere
I think (?). Let me know if there are more places that I missed.

> [3]
>
> > One thing to note is that this needs to go into Trusty as I understand it, so it probably needs to go in 1.5 as well.
> > I'm not exactly sure how to lp-fu that, so if someone has some instrucitons, that would be great :-)
>
> Once this is landed, create a branch off lp:maas/1.5, cherry pick the revision where this branch has landed: bzr -c <rev_number> lp:maas then create a MP for this branch with lp:maas/1.5 as the target.

Excellent, thank you.

\t