Arik Hadas has posted comments on this change.

Change subject: API: modernize VM methods
......................................................................


Patch Set 18:

(1 comment)

https://gerrit.ovirt.org/#/c/61475/18/vdsm/API.py
File vdsm/API.py:

Line 263:         """
Line 264:         Lock user session in guest operating system using guest agent.
Line 265:         """
Line 266:         self.vm.guestAgent.desktopLock()
Line 267:         if self.vm.guestAgent.isResponsive():
> Would using a variable really make the code less readable? Not that importa
+1
I personally find:

vm = self.vm
vm.guestAgent.desktopLock()
if vm.guestAgent.isResponsive():

to be as easy to read as lines 266-267 above and less confusing since calling 
vm() twice could make people wonder "why the hell do they query the VM twice? 
can the VM change in the middle?"
but I don't really mind - it is really not that important here..
Line 268:             return {'status': doneCode}
Line 269:         else:
Line 270:             return errCode['nonresp']
Line 271: 


-- 
To view, visit https://gerrit.ovirt.org/61475
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e2e238fc632df97b63f7bb2a6293fe1c392a842
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Irit Goihman <igoih...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Reply via email to