David Caro has posted comments on this change. Change subject: ovirt-live: migrate ovirt live plugin to otopi ......................................................................
Patch Set 2: (2 inline comments) .................................................... File fedora/oVirtLiveFiles/root/usr/share/ovirt-engine/setup/plugins/ovirt-engine-setup/olive/core.py Line 1: # Encoding should be present even if it's not executed directly, and shebang does not hurt. Usually I prefer putting it always even if is not needd than missing it once when it was. Line 2: # ovirt-engine-setup -- ovirt engine setup Line 3: # Copyright (C) 2013 Red Hat, Inc. Line 4: # Line 5: # Licensed under the Apache License, Version 2.0 (the "License"); Line 111: type='data') Line 112: Line 113: engine_api.storagedomains.add(sdParams) Line 114: Line 115: engine_api.datacenters.get(self.environment[oliveconst.ConfigEnv.DEFAULT_LOCAL_DATA_CENTER]).storagedomains.add(engine_api.self.environment[oliveconst.ConfigEnv.DEFAULT_ISO_NAME]) See this example: dc_name = oliveconst.ConfigEnv.DEFAULT_LOCAL_DATA_CENTER storage_name = oliveconst.ConfigEnv.DEFAULT_ISO_NAME dc = engine_api.datacenters.get(self.environment[dc_name]) dc.storagedomains.add(self.environment[storage_name]) Is a lot more easy to understand that reading it all in one don't you think? Breaking all the logic to small ordered steps helps the reader follow the flow in my opinion. Line 116: Line 117: @plugin.event( Line 118: stage=plugin.Stages.CLOSEUP, Line 119: condition=lambda self: self._enabled, -- To view, visit http://gerrit.ovirt.org/17518 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I703f64dc1183a6fe176d9d0352f93de381d906bb Gerrit-PatchSet: 2 Gerrit-Project: ovirt-live Gerrit-Branch: master Gerrit-Owner: Ohad Basan <oba...@redhat.com> Gerrit-Reviewer: Alex Lourie <alou...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: David Caro <dcaro...@redhat.com> Gerrit-Reviewer: Eyal Edri <ee...@redhat.com> Gerrit-Reviewer: Itamar Heim <ih...@redhat.com> Gerrit-Reviewer: Moran Goldboim <mgold...@redhat.com> Gerrit-Reviewer: Ohad Basan <oba...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
