Code review comment for lp:~wgrant/launchpad/refactor-slavestatus

William thanks for the fix. The logic of what you have done looks good.

However, I don't like the idiom you've employed of mutating the status dictionary inside the slaveStatus methods and then returning it. If you didn't want to mutate the original dictionary then making a copy, changing it, and returning that copy would make sense. Otherwise just modifying the status dictionary in place and not returning it is a reasonable thing to do (and preferred barring no solid reason to do the former). But to mutate and then return would lead one to think the dictionary was not modified in place, which we know it was.

Changing the method to mutate will necessitate a small change to your new test code.

I'm marking this approved rather than needs fixing with the expectation you'll make that change. If you would prefer to discuss it further we can and I'd be eager to hear your thoughts.

As usual, I'm happy to land this for you when it is ready.