Yedidyah Bar David has posted comments on this change. Change subject: packaging: setup: Clean up treatment of /etc/exports ......................................................................
Patch Set 5: (2 comments) .................................................... File packaging/setup/plugins/ovirt-engine-setup/system/exportfs.py Line 118: if os.path.exists(osetupcons.FileLocations.NFS_EXPORT_DIR): Line 119: self._conf = osetupcons.FileLocations.OVIRT_NFS_EXPORT_FILE Line 120: if index is not None: Line 121: exports_changed = True Line 122: new_line = exports_content[index] I'll keep both. Naturally we do not know if it works. But if it's just default of legacy it should work, and if it was manually changed later by the admin, I'd rather it fails and be manually fixed again, and then saved for future upgrades. Line 123: del exports_content[index] Line 124: self.environment[otopicons.CoreEnv.MAIN_TRANSACTION].append( Line 125: filetransaction.FileTransaction( Line 126: name=self._conf, Line 128: modifiedList=uninstall_files, Line 129: ) Line 130: ) Line 131: else: Line 132: self._conf = osetupcons.FileLocations.NFS_EXPORT_FILE Really? if cond1: a = value1 else: a = value2 # bla bla bla if a == value1: stuff else: other stuff is better than: if cond1: a = value1 stuff else: a = value2 other stuff ? Especially that there is other code referring to both value1 and value2? It's not as if we set a according to cond1 and then forget about it. And, btw, self._conf is only used one time later, in the note in closeup. Line 133: if index is None: Line 134: exports_changed = True Line 135: exports_content.append(new_line) Line 136: exports_uninstall_group.addChanges( -- To view, visit http://gerrit.ovirt.org/20709 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I035817c048518f77fc666cc6d5212c8e46edd65d Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
