Code review comment for lp:~rconradharris/nova/uuid-redux
Revision history for this message
| Rick Harris (rconradharris) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
35a32c5
(Get the code!)
> 8-37, 92: This is good refactoring, but it doesn't seem directly related to this branch. I'm not asking you to remove it, I just think we shouldn't touch things like this until we have a clear reason to change it.
The intent here wasn't to refactor, that was just a by-product. There was a nasty bug in the code that we uncovered. Notice, `server_id` and `id` in those methods. Since `id` wasn't passed in, it was actually using python's built-in function `id`.
The refactoring just made the cleanup a little easier.
(As to why the tests passed before, no idea. They shouldn't have. Maybe due to the shear amount of fakes and stubs.)
> 178: This method name is a bit misleading. Can you call it _build_
Okay.
> Is there a future plan to either remove instance.id or replace it with the uuid value? I can understand why it isn't in this MP.
I think that's in the long-term plan; but, definitely couldn't get that in one-shot. This gets a step-closer and allows us to move to UUID in a careful, piece-wise fashion.