Fabian Deutsch has posted comments on this change. Change subject: Add a confirmation page ......................................................................
Patch Set 5: Code-Review-1 (2 comments) http://gerrit.ovirt.org/#/c/27074/5/src/ovirt/node/installer/core/confirmation_page.py File src/ovirt/node/installer/core/confirmation_page.py: Line 34: super(Plugin, self).__init__(app) Line 35: self.storage_discovery = StorageDiscovery(app.args.dry) Line 36: self.storage_discovery.start() Line 37: self._header = "{!s:8.8} {!s:48.48} {!s:9.9}" Line 38: self._tagged_pvs = [x.split()[0] for x in Please move some of this code into a class, to make it more programatically accessible (will be easier to add unit-tests/doctests). utils/system.py class LVM(…): def vgs(self): return [LVM.VG( class VG(…): def tags(self): return … then def is_tagged(name): if name in LVM().vgs() and "storage_domain" LVM.vg(name).tags() or something along this lines. Mainly to keep much of the low-level logic in the utils class. Basically split business/domain knowledge (if tagged storage_domain, then it's in use) here, and the low-level/generic helper stuff in the utils/* classes Line 39: process.check_output(["lvm", "vgs", Line 40: "--noheadings", Line 41: "-o", "pv_name,tags"]).split( Line 42: "\n") if "storage_domain" in x] Line 60: align = lambda l: l.ljust(16) Line 61: if not self._model: Line 62: self._build_model() Line 63: Line 64: ws = [ui.Header("header[0]", _("Confirm disk selections")), Maybe we should add a more inforamtionl text like "Please review this page carefully. The data on the disks below will be erased" Or something like this. you can also set the UX keyword on the bug and someone from teh UX team can take a look. Line 65: ui.Header("boot.header", _("Boot device")), Line 66: DiskDetails("boot.device", self, Line 67: self._model["boot.device.current"])] Line 68: -- To view, visit http://gerrit.ovirt.org/27074 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If0c099fbfbfda9bc609a024964905ee59e04280f Gerrit-PatchSet: 5 Gerrit-Project: ovirt-node Gerrit-Branch: master Gerrit-Owner: Ryan Barry <[email protected]> Gerrit-Reviewer: Fabian Deutsch <[email protected]> Gerrit-Reviewer: Ryan Barry <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ node-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/node-patches
