Martin Peřina has posted comments on this change. Change subject: core: monitoring: adhere to fields naming convention ......................................................................
Patch Set 12: (5 comments) http://gerrit.ovirt.org/#/c/27941/12/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/HostMonitoring.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/HostMonitoring.java: Line 48: @SuppressWarnings({ "synthetic-access", "unchecked", "rawtypes" }) Line 49: public class HostMonitoring { Line 50: private final VDS vds; Line 51: private final VdsManager vdsManager; Line 52: private VDSStatus firstStatus = VDSStatus.forValue(0); Why do you initialize here, when you overwrite this value in constructor? Line 53: private final MonitoringStrategy monitoringStrategy; Line 54: private boolean saveVdsDynamic; Line 55: private boolean saveVdsStatistics; Line 56: private boolean processHardwareCapsNeeded; Line 53: private final MonitoringStrategy monitoringStrategy; Line 54: private boolean saveVdsDynamic; Line 55: private boolean saveVdsStatistics; Line 56: private boolean processHardwareCapsNeeded; Line 57: private boolean refreshedCapabilities = false; Please also move initialization to constructor Line 58: private VmsMonitoring vmsMonitoring; Line 59: private static Map<Guid, Long> hostDownTimes = new HashMap<>(); Line 60: private boolean vdsMaintenanceTimeoutOccurred; Line 61: private Map<String, InterfaceStatus> oldInterfaceStatus = new HashMap<String, InterfaceStatus>(); Line 57: private boolean refreshedCapabilities = false; Line 58: private VmsMonitoring vmsMonitoring; Line 59: private static Map<Guid, Long> hostDownTimes = new HashMap<>(); Line 60: private boolean vdsMaintenanceTimeoutOccurred; Line 61: private Map<String, InterfaceStatus> oldInterfaceStatus = new HashMap<String, InterfaceStatus>(); Please also move initialization to constructor Line 62: Line 63: private static final Logger log = LoggerFactory.getLogger(HostMonitoring.class); Line 64: Line 65: public HostMonitoring(VdsManager vdsManager, VDS vds, MonitoringStrategy monitoringStrategy) { http://gerrit.ovirt.org/#/c/27941/12/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmsMonitoring.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmsMonitoring.java: Line 102: private final List<Guid> autoVmsToRun = new ArrayList<>(); Line 103: private final List<VmStatic> externalVmsToAdd = new ArrayList<>(); Line 104: private final Map<Guid, VmJob> vmJobsToUpdate = new HashMap<>(); Line 105: private final List<Guid> vmJobIdsToRemove = new ArrayList<>(); Line 106: private final List<Guid> existingVmJobIds = new ArrayList<>(); Please move all these attribute initialization into constructor Line 107: Line 108: private static final Map<Guid, Integer> vmsWithBalloonDriverProblem = new HashMap<>(); Line 109: private static final Map<Guid, Integer> vmsWithUncontrolledBalloon = new HashMap<>(); Line 110: Line 115: Line 116: /** names of fields in {@link VmDynamic} that are not changed by VDSM */ Line 117: private static final List<String> UNCHANGEABLE_FIELDS_BY_VDSM; Line 118: Line 119: static { When this static block exist, wouldn't it be better to execute all static attributes initialization inside it? Line 120: List<String> tmpList = new ArrayList<String>(); Line 121: for (Field field : VmDynamic.class.getDeclaredFields()) { Line 122: if (field.isAnnotationPresent(UnchangeableByVdsm.class)) { Line 123: tmpList.add(field.getName()); -- To view, visit http://gerrit.ovirt.org/27941 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I18d520d3b7a61c2e02bb43ca73334b8c0ecc1c23 Gerrit-PatchSet: 12 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Roy Golan <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
