launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

This change looks sane to me.

[0]

As you say, let's test this before we merge it.  Also, we need to wait for the fix to python Juju to be released to land this; otherwise, if I understand this correctly, the new test won't pass.

[1]

8	+    def test_15A_apply_tag(self):

This '15A' is a bit ugly but Diogo is working on using nose's ability declarative state in which order the tests must be executed so let's land this as is and we will fix it when the Diogo's change will land.

[2]

8	+    def test_15A_apply_tag(self):

Still, s/apply_tag/apply_tag_to_all_nodes/ would be more descriptive.

[3]

14	+        # Really it might take a while for tags to get applied, so waiting
15	+        # for all the nodes to be listed from 'tags' here might make sense.

I think, we can do that easily using the same kind of code that '_wait_nodes' uses.
-- 
https://code.launchpad.net/~gz/maas/qa-lab-tests-maas-tags/+merge/149652
Your team MAAS Maintainers is requested to review the proposed merge of lp:~gz/maas/qa-lab-tests-maas-tags into lp:~maas-maintainers/maas/qa-lab-tests.