Nir Soffer has posted comments on this change. Change subject: packaging: setup: SANWipeAfterDelete configuration ......................................................................
Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/37397/1/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/storage.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/config/storage.py: Line 50: condition=lambda self: Line 51: self.environment[oenginecons.EngineDBEnv.NEW_DATABASE], Line 52: ) Line 53: def _configureSANWipeAfterDelete(self): Line 54: sanWipeAfterDelete = dialog.queryBoolean( > I disagree. You assume that this function can perform multiple queries, so each query should be named. But performing multiple queries here is wrong - a function should do only one thing. If you need another query, do each one in a separate function. Again, you are optimizing for the wrong use case - "what if someone will have to add another query, she will have to rename value to sanwipeafterdelete". The code should be optimized for what is needed now, not what may be needed 2 years from now. If we think about the future, we don't want to make it easy to go and add another query here, because we don't want to maintain long functions. Anyway this is not critical. Line 55: dialog=self.dialog, Line 56: name='OVESETUP_CONFIG_SAN_WIPE_AFTER_DELETE', Line 57: note=_( Line 58: 'SAN wipe after delete ' -- To view, visit http://gerrit.ovirt.org/37397 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I911f87ad34eafc83eb2c7aa346fc4278ada28b5b Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Candace Sheremeta <csher...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
