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

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

-----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-----

review: Approve

« Back to merge proposal