Code review comment for lp:~mwhudson/ubuntu-cdimage/netboot-tarball

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

> In general this feels good.

Thanks!

> Of course the re-tarring of the tarball is a bit
> controversial, but makes sense in this case - especially that per the
> specification we also want the contents to be untarred in the netboot/
> directory anyway.

Yeah, that's definitely feels a bit unusual. One consequence is that we'll have to do it again when doing the release publication -- not sure the code is set up for that though, does it recompute checksums as part of release publication? (I guess given that in general, not every arch gets published, it must?)

> Left a few inline comments with some things that need changing though. Another
> thing that needs fixing is: we need unit tests for this new case. The cdimage
> codebase has usually quite a good test suit, so whenever we add something new,
> we should make sure it's tested as well. This way we can know if it works-ish.

Good point. I've added test cases to test_livefs.py and test_tree.py. Couldn't figure out how to test the build.py change!

« Back to merge proposal