Code review comment for lp:~allenap/maas/repackage

Plowing right on! Looking forward to getting this in.

In pkg/maas-region/maasserver/components.py, line 42 or so, you have a docstring, then a function call, and *then* an import that's there to avoid an import cycle. Would be more regular and obvious to have the import right below the docstring:

3328 :param error_message: Human-readable error text.
3329 """
3330 discard_persistent_error(component)
3331 + from maasserver.models import ComponentError # Avoid circular imports.
3332 ComponentError.objects.create(component=component, error=error_message)

.

The comment in MAASServerTestCase.setUp really doesn't need to have a TODO marker as far as I'm concerned. It's a great comment, but it does not instill a great sense of urgency. :-)

.

In pkg/maas-region/maasserver/tests/test_views_nodes.py, why does test_node_list_sorts_by_zone newly skip with an XXX message from gmb? It looks as if you were just porting over a previous skip() to a better format, but I don't see anything that would have made that test skip before.

.

The documentation for ClassSetup relies on knowledge of MAASTestCaseType in a different module. Can you explain it in such a way that people who aren't aware of MAASTestCaseType's relevance have a chance of understanding it? Can you also explain in docstrings *why* MAASTestCaseType forbids setUpClass if we then need ClassSetup to un-forbid it again?