Alissa Bonas has posted comments on this change.
Change subject: findbugs: Fix field should be package protected warning in
uicommonweb project
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterPolicyModel.java
Line 13:
Line 14: @SuppressWarnings("unused")
Line 15: public class ClusterPolicyModel extends EntityModel {
Line 16:
Line 17: static Integer lowLimitPowerSaving = null;
Why those members should be package protected? What's the benefit and the
purpose of it in this specific class? Any other class using it right now? It's
so fragile to rely on package protected access.
Line 18: static Integer highLimitPowerSaving = null;
Line 19: static Integer highLimitEvenlyDistributed = null;
Line 20:
Line 21: private EntityModel privateOverCommitTime;
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmInterfaceModel.java
Line 37: @SuppressWarnings("unused")
Line 38: public abstract class VmInterfaceModel extends Model
Line 39: {
Line 40: public final static String ENGINE_NETWORK_NAME =
Line 41: (String)
AsyncDataProvider.GetConfigValuePreConverted(ConfigurationValues.ManagementNetwork);
Why changing it to final and moving the initialization from constructor here?
What is gained by this? What if the call to AsyncDataProvider during class
initialization will fail - very hard to investigate.
Also, this is not related to the patch title which says something about package
protected warnings...
Line 42:
Line 43: private boolean privateIsNew;
Line 44: private EntityModel privateName;
Line 45: private ListModel privateNetwork;
--
To view, visit http://gerrit.ovirt.org/14477
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I54b79981eb3732232f351f67809a6c622fdc1da2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
