Francesco Romani has posted comments on this change. Change subject: virt: Make VM domain descriptor public ......................................................................
Patch Set 1: Code-Review-1 (1 comment) -1 for visibility. I think we can have a much simpler patch using a property (or explicit getter, see inline comment). https://gerrit.ovirt.org/#/c/53371/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 291: self.domain = DomainDescriptor.from_id(self.id) I agree with Nir's comment. This could be public, and having it public indeed makes some flows better. But it should not be writeable, so we can do either below: @property def domain(self): return self._domain or we can just have a method called domain(). This also makes this patch much shorter and easier to review. Milan raised the concern that we may overuse properties. This could be true, but I think in this case a read only property fits nicely our use cause. -- To view, visit https://gerrit.ovirt.org/53371 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I58dacec299fd6176b10b4180e2737d6b2f59ccbc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Betak <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
