Code review comment for lp:~rackspace-titan/nova/instance_states

Revision history for this message
Brian Waldon (bcwaldon) wrote :

This is absolutely incredible. You've done a great job, here. One general comment before the line-by-line stuff:

Can we align the states with respect to tense? I don't think all of our states need to end in ING or ED. What do you think?

16: I think you might want to expand this comment to explain yourself better better

53/54: Should this be 'stopped' and 'terminated'? According to the allowed EC2 values, these don't seem correct.

189: Can you log the output of the command you allude to?

955/990: You should probably file a bug for this, seems like a simple cleanup item

1224: Thank you for cleaning this function up.

1335/1390: I would love to expand on these module-level docstrings. I think adding a bit more context/explanation of what the task/vm_states represent would be very helpful.

1458/1471: Not sure what these comments mean...

review: Needs Fixing

« Back to merge proposal