Allon Mureinik has posted comments on this change.

Change subject: webadmin: Configurable Default for Wipe After Delete per 
Storage Domain
......................................................................


Patch Set 5: Code-Review+1

(1 comment)

+1 on the backend+test.

Can't spot any glaring issues with the frontend, but a better trained eye 
should review.

http://gerrit.ovirt.org/#/c/36526/5/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetStorageDomainWipeAfterDeleteQueryTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetStorageDomainWipeAfterDeleteQueryTest.java:

Line 21:     @Before
Line 22:     @Override
Line 23:     public void setUp() throws Exception {
Line 24:         super.setUp();
Line 25:         mcr.mockConfigValue(ConfigValues.SANWipeAfterDelete, true);
There's a race between this patch and http://gerrit.ovirt.org/#/c/36539/.

If http://gerrit.ovirt.org/#/c/36539/ is merged first, this block should be 
redone to use getExtraConfigDescriptors().
If this ones gets merged first, please -1 http://gerrit.ovirt.org/#/c/36539/ as 
it does not handle this query as it should
Line 26:     }
Line 27: 
Line 28:     @DataPoints
Line 29:     public static StorageType[] types = StorageType.values();


-- 
To view, visit http://gerrit.ovirt.org/36526
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id0bf6a95eb4b33867de86e99f8cb59eec0d1cdd4
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ish...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Idan Shaby <ish...@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

Reply via email to