Code review comment for lp:~gmb/maas/json-schema-part-1

Revision history for this message
Julian Edwards (julian-edwards) wrote :

On Monday 10 Feb 2014 12:06:29 you wrote:
> > (FWIW My vim plugin reads the :type: and offers better auto-completions :)
>
> Which vim plugin loveliness is it of which you speak? Fixed.

https://github.com/Valloric/YouCompleteMe

It is seriously *awesome*.

> > [2] I realise it's early days, but the FIELD_TYPE_MAPPINGS are going to
> > form part of the API with the drivers. I think putting these in their
> > own module would be better (like errno), so we can do something like
> > powerfield.mac_address, for example. This also then forms a firm contract
> > with the driver, which consumes the API. (It also makes it easier to
> > install for packaging and is something we should have done for enums so
> > that maas api clients can use them)
>
> Okay, I'm a bit lost as to what you mean here... I was with you up
> until "their own module" but then you started talking about packaging
> and I got confused. Are you talking about just adding a module under
> src/maasserver (which is what I think you mean)?

Ok so these are a bit like the enums in that third party code (drivers) are
going to need access. Putting it in its own module will help.

For packaging, I was thinking more about maas-cli because we could install an
enums.py (for example) along with the cli itself. This doesn't translate too
well for the drivers since they are going to be part and parcel of maas
itself; it's just that we want to make life as easy as possible for driver
writers so I'd prefer if we had all things that drivers will need to import in
a separate and convenience place.

Hope that makes sense :)

> Anyway, in the interests of getting this landed I'll do that in a
> followup branch once I've got clarity.

Naw worries.

It could probably even wait until Gavin starts writing the API layer.

> Fixed. Cheers for the review.

Pleasure.

« Back to merge proposal