Allon Mureinik has posted comments on this change. Change subject: vm: Unify checks for vdsm image ......................................................................
Patch Set 3: Code-Review+1 (4 comments) .................................................... Commit Message Line 6: Line 7: vm: Unify checks for vdsm image Line 8: Line 9: When testing if dict or vm.Drive object are vdsm image, we have to do Line 10: ugly type check, and then invoke either vm.Drive.isVdsmImage, or s/ugly/an ugly/ Line 11: vm.isVdsmImage, which use different logic. Line 12: Line 13: Having vm.Drive.isVdsmImage looks first as an improvement, but since we Line 14: handle both dicts and drive objects, this only complicates the code. The Line 9: When testing if dict or vm.Drive object are vdsm image, we have to do Line 10: ugly type check, and then invoke either vm.Drive.isVdsmImage, or Line 11: vm.isVdsmImage, which use different logic. Line 12: Line 13: Having vm.Drive.isVdsmImage looks first as an improvement, but since we s/looks first/looks, at first glance,/ Line 14: handle both dicts and drive objects, this only complicates the code. The Line 15: different logic creates confusion and lead to pointless discussions if Line 16: some key is needed or not in certain context. Line 17: Line 11: vm.isVdsmImage, which use different logic. Line 12: Line 13: Having vm.Drive.isVdsmImage looks first as an improvement, but since we Line 14: handle both dicts and drive objects, this only complicates the code. The Line 15: different logic creates confusion and lead to pointless discussions if s/lead/leads/ Line 16: some key is needed or not in certain context. Line 17: Line 18: This patch unify this check; both vm.Drive and dict have now similar Line 19: interface so isVdsmImage can handle both, and we use the same logic. Line 14: handle both dicts and drive objects, this only complicates the code. The Line 15: different logic creates confusion and lead to pointless discussions if Line 16: some key is needed or not in certain context. Line 17: Line 18: This patch unify this check; both vm.Drive and dict have now similar s/unify this check/unifies these checks/ Line 19: interface so isVdsmImage can handle both, and we use the same logic. Line 20: Line 21: Change-Id: Iaa109a19aeec56f09ed2e6d64dee636c7779d426 -- To view, visit http://gerrit.ovirt.org/22370 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaa109a19aeec56f09ed2e6d64dee636c7779d426 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
