Code review comment for lp:~allenap/maas/remove-squashfs

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

> > 1. One bug report I see linked is server side and is *not* An ssue with this
> > feature.
> > 2. Another report is an issue of not having updated a file with the release
> we
> > want to import which is not even related to the script maas-import-squashfs
> > but also affects maas-import-pxe-files and maas-import-ephemerals. and this
> > bug would have still being there even of the feature wouldnt have existed.
> > 3. Legitimate maas-import-squashfs.
>
> Removing squashfs support means we can close two of these bugs, and
> the third becomes a trivial fix.

Closing 1 doesn't change the fact that it is still an issue om the server side. If for some reason the PXE images would be unavailable from the server, then the same bug would apply to maas-import-pxe-files, or to maas-import-ephemerals for such case. That's even if we didn't have the support for squashfs. Either way, the bug reported by Diogo was only seen in the lab that maybe had bad mirrors, because it was proven that at other locations the bug wasn't present.

2 is a bug which is unrelated with squashfs support because this bug affects maas-import-pxe-files and maas-import-ephemerals. Removing the support doesn't fix it, because the bug will still be present even if there was no squashfs support. In fact, there's was a bug in MAAS that affected installations using squashfs, not because of the support but because of a bug in MAAS.

3. Simple fix.

>
> For the size and benefit of the feature - i.e. a ~1 minute speedier
> install - it's attracting a disproportionately high number of
> bugs. That's my impression - I haven't tried to quantify it - but I'm
> not alone in thinking this.

2 minute to be exact.

> What is broken is the lack of tests. "Untested code is broken code" is
> a principle that the Launchpad teams have all tried to follow for
> years. It would be good to reintroduce this feature later on, or FPI,
> but not in its current form.

Right, but I was just pointing out that it isn't the only thing untested that maas ships, is it? However, its been placed as a strong reason for dropping the support.

>
> Bugs filed is not a measure of quality.

Exactly, number of bugs is not a measure of quality; yet, the statement for removal of the feature is based on "disproportionate number of bugs" and "doesn't work by most/all accounts", which are both based and backed by 3 bug reports linked to this MP. However, both statements IMHO are incorrect.

Cheers

« Back to merge proposal