Alon Bar-Lev has posted comments on this change.

Change subject: ovirt-live: migrate ovirt live plugin to otopi
......................................................................


Patch Set 2: (3 inline comments)

....................................................
File 
fedora/oVirtLiveFiles/root/usr/share/ovirt-engine/setup/plugins/ovirt-engine-setup/olive/core.py
Line 1: #
No need in python module.... this is not executed directly.
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 29: 
Line 30: from otopi import util
Line 31: from otopi import plugin
Line 32: from otopi import filetransaction
Line 33: from otopi import constants as otopicons
I, personally, find it less readable.
Line 34: 
Line 35: import oliveconst
Line 36: 
Line 37: from ovirt_engine_setup import constants as osetupcons


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])
I actually against intermediate variables, it does not improve readability. 
however style will be nice:

In this example:

 engine_api.datacenters.get(
     self.environment[
         oliveconst.ConfigEnv.DEFAULT_LOCAL_DATA_CENTER
     ]
 ).storagedomains.add(
     engine_api.self.environment[
         oliveconst.ConfigEnv.DEFAULT_ISO_NAME
     ]
 )

you get the datacenter and execute the add, single statement no intermediates, 
just clear style.
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

Reply via email to