Yaniv Bronhaim has posted comments on this change.
Change subject: Adding supervdsm unit tests
......................................................................
Patch Set 8: (2 inline comments)
....................................................
File vdsm/supervdsm.py
Line 90
Line 91
Line 92
Line 93
Line 94
I don't think that putting a comment here can help better to understand it..
It is just confusing.
First, I add the comment in the patch description that explains how those
overidding works.
Second, you ask for comment that related to the tests inside the
implementation code. It sounds redundant, maybe I should put this comment
inside the tests setUp function, but I still think that comment is not needed
here.. You can easily understand what the comment that you want says by
following the tests code.
And third, you ask for ## # These variables are overridden during testing. If
you make changes to them # be careful to maintain the option of creating a test
instance ##
What changes? like new tests?
Maybe we can phrase a better comment if we find it important..
....................................................
File vdsm/supervdsmServer.py
Line 376:
Line 377: log.debug("Started serving super vdsm object")
Line 378: servThread.join()
Line 379: except Exception as e:
Line 380: log.error("Could not start Super Vdsm: %s", e)
What was it before?... I don't understand your comment.
I found it better not to print the stacktrace here because there is no stack
here, its the main function.
You can ask why I did this before giving it -1... and if you still think that
this is wrong please explain why - 'undesirable exception printing method' is
not an explanation.. why is it undesirable for you?
Line 381: sys.exit(1)
Line 382: finally:
Line 383: try:
Line 384: if address is not None and os.path.exists(address):
--
To view, visit http://gerrit.ovirt.org/8600
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I08880b12db8a6ca9cc57d74d2ef2a86980a5b097
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
