Code review comment for lp:~julian-edwards/maas/consider-static-range

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

"Everything is closely tied" is not a reason not to break things up; it's a reason to put more effort into breaking them up! Such irredeemable patches of code tend to spread like oil slicks. There's clearly stuff here that could be extracted, and make the code cleaner for it. I see loops that could do with a name, a docstring, or at least a comment.

One question about the design of this branch: why would you look just at the static range, and not at the network's full address range? Does it matter for the purpose of linking a node's network interface to a subnet whether the address is in the static range? If I reduced the static range after a node had already acquired a static address, so that the address is now outside the static range, should that affect whether this could makes the link? Wouldn't be a check against the network be more robust, as well as faster (because IPRange is so awful about “in”) and simpler?

review: Approve

« Back to merge proposal