Code review comment for lp:~blake-rouse/maas/add-osystem-to-bootimage

Revision history for this message
Andres Rodriguez (andreserl) wrote :

BTW... How will this affect upgrades? My concern is that if we backport
this after upgrades things will be broken because the location of the
bootimages has now changed right? What should we do about it? Julian, Gavin
any thoughts?
On Apr 23, 2014 11:06 PM, "Julian Edwards" <email address hidden>
wrote:

> Review: Approve
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> review: approve
>
> Wow, adding something as simple as an OS name was quite a lot of
> changes! Thanks for doing this, it's a very nice branch.
>
> I have a couple of niggles but it looks mostly OK to me. Please land
> it once these are addressed.
>
> [1]
>
> > + def get_usable_osystems(self, nodegroup): + """Return
> > the list of usable operating systems for a nodegroup. + """
> > + query = BootImage.objects.filter(nodegroup=nodegroup) +
> > return set(query.values_list('osystem', flat=True)) + + def
> > get_usable_releases(self, nodegroup, osystem): + """Return
> > the list of usable releases for a nodegroup and + operating
> > system. + """ + query =
> > BootImage.objects.filter(nodegroup=nodegroup, osystem=osystem) +
> > releases = query.values_list('release', flat=True) + return
> > set(releases)
>
> Can you add tests for these new functions please. Also they are not
> actually used anywhere, I presume a follow up branch makes use of them?
>
> [2]
>
> Please run "make lint" and fix all the errors it finds.
>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlNYfxYACgkQWhGlTF8G/HefwwCfWwiomUOBgXKO3Z4mf/YGEfpL
> VaEAniGAr/MpfihFXHwJBKOekqD2KGOp
> =VxoG
> -----END PGP SIGNATURE-----
>
> --
>
> https://code.launchpad.net/~blake-rouse/maas/add-osystem-to-bootimage/+merge/216923
> You are subscribed to branch lp:maas.
>

« Back to merge proposal