Francesco Romani has posted comments on this change.

Change subject: v2v: Lazy loading of external VMs info
......................................................................


Patch Set 1:

(1 comment)

initial review.
Have you explored the possibility of adding a new verb instead of adding more 
parameters to this one?
If so, could you elaborate why you choose this way?
(trying to understand the issue and process, not questioning the implementation 
now)

https://gerrit.ovirt.org/#/c/57418/1/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 413:         if s is None:
Line 414:             return False
Line 415:         if type(s) == bool:
Line 416:             return s
Line 417:         if str(s).lower() == 'true':
> Why do you need this change?
+1
If you really need this, it belongs to a separate patch.
Line 418:             return True
Line 419:         return bool(int(s))
Line 420:     except:
Line 421:         return False


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3dc6fc4573b2c0b1c03ed87025452e14af2fc566
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky <tgole...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com>
Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com>
Gerrit-Reviewer: Tomas Golembiovsky <tgole...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to