Alona Kaplan has posted comments on this change. Change subject: webadmin: edit vfsConfig- persist vfs config changes ......................................................................
Patch Set 32: (1 comment) https://gerrit.ovirt.org/#/c/36421/32/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostNicModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostNicModel.java: > Correct me if I'm wrong, but it seems no changes in this file are needed if You're right, things will work without the change. But I prefer to keep it as is for some reasons- 1. Consistency with vfsConfigModel. 2. The view shouldn't work with the business entities, it should work with the model. IMO, it is bad design for the view the check if the interface is bond slave. The view should just know if the HostNicModel contains NicLabelModel or not. The best way to do it is to init the labelsModel with null value. But because of GWTP restrictions it is not possible (it fails in the initialization level). So, the next best solution is to use an empty state model. 3. Readability and avoiding future bugs. There are two use cases, HostNicModel code should reflect it. Line 1: package org.ovirt.engine.ui.uicommonweb.models.hosts; Line 2: Line 3: import java.util.Collection; Line 4: import java.util.Collections; -- To view, visit https://gerrit.ovirt.org/36421 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa221586d123a3bc4925b077921391508a4680ec Gerrit-PatchSet: 32 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
