Code review comment for lp:~blake-rouse/maas/new-images-page

Revision history for this message
Blake Rouse (blake-rouse) wrote :

> Looks really good so far. I was able to merge lp:~ltrager/maas/use_bootloaders
> into your branch to test it with the bootloaders.

Thanks for looking at it.

>
> Couple of things I noticed
> 1. The bootloaders still show up. In my branch I filter them with
> BootSourceCache.objects.filter(bootloader_type=None)

Ah thats much better. I cannot do that until your branches make it into trunk so I will leave them as so.

> 2. The other images section lists the images twice, onces with checkboxes,
> onces without. See http://i.imgur.com/EJOjhpg.png

This is correct. You select the images you want then save that selection and they will be imported. At the moment yours look wierd and is confusing since you see the bootloaders. With the change above it will make more since.

> 3. I started with MAAS using the V3 test stream. If I click on the maas.io
> source the boot source metadata is immediately reloaded. It looks like the
> region then tries to download the maas.io images(they're at an older version
> than mine). If I switch back to my custom stream 16.10 is marked 'Queued for
> deletion.' I see there is a 'connect' button. I think the user should have to
> click connect before things change. This should prevent downloading the image
> again if the source is accidentally changed.

I think I understand what you are describing and I have cleaned this up. It doesn't actually start downloading. You have to click save selection, for it to be saved.

> 4. Sometimes when the page loads its blank or I only see other images.
> Refreshing fixes this.

Yeah I have fixed this issue.

> 5. When I click connect with a custom stream I briefly(only shows for about a
> second) get an error message saying 'Select atleast one 14.04+ LTS release and
> architecture' even though I already have 16.04 installed.

What you have installed doesn't matter since your on a new stream. You have to make selections for that new source.

« Back to merge proposal