Code review comment for lp://qastaging/~jordanrinke/nova/hypervupdate

Revision history for this message
Soren Hansen (soren) wrote :

Yes, we do not want everything to be executable, because... well, they aren't. Attempts to run the vast majority of these things will blow up in your face (no #! magic to point it to a python interpreter). Please fix that.

You've added a duplicate "import sys" in nova-compute

> +class compute_win32_service(win32serviceutil.ServiceFramework):

Is there any parts of the win32serviceutil framework that forces a specific naming scheme for classes? If not, please change the class name to match our usual style ("ComputeWin32Service").

> + #Trying to use code directly from vish/libvirt for portability

I don't understand this comment?

Also, some unit tests for all of this would be really nice. I'm have no Windows machines for testing, so I have no way to know if I'll be breaking your code.

review: Needs Fixing

« Back to merge proposal