Code review comment for lp:~blake-rouse/maas/backward-compatible-boot-source-api

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Excellent stuff. My eyes did glaze over a bit towards the end, but sometimes there's just no helping that with API tests.

I try always to find something to consider changing, just to prove that I'm really reviewing the branch, so here goes:

.

The docstring for BootSourcesBackwardHandler.read explains the uuid parameter, but maybe it should just say that it's a cluster uuid that is now ignored?

.

It looks like test_GET_returns_same_boot_source_for_different_node_groups and the two instances of test_GET_returns_same_list_for_different_node_groups each test the same thing 3 times. Does doing it more than once really contribute anything?

review: Approve

« Back to merge proposal