launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

> [0]
> 
> 79      +    def get_nodegroup_uri(self, nodegroup):
> 80      +        """Compose the API URI for `nodegroup`."""
> 81      +        return self.get_uri('nodegroups/%s/' % nodegroup.name)
> 
> You can use 'reverse' instead of creating that custom method.  This is simpler
> and has the advantage of also testing the  " name='nodegroups_handler'" part
> in the url(...) declaration.

Thanks, that's much better.  Not that I consider “also testing” a good thing: integration-testing shouldn't be implicit in unit tests!


> [1]
> 
> Prerequisite:   lp:~jtv/maas/nodegroups-api
> 
> Unless I'm mistaken, the diff shows the modifications in the pre-req as well
> :(.

Yes, see the merge proposal.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/nodegroup-update-lease-api/+merge/114565
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/nodegroup-update-lease-api into lp:maas.