Juan Hernandez has posted comments on this change. Change subject: restapi: Add per-VM migration_downtime to REST API ......................................................................
Patch Set 9: (2 comments) Loogs good, but I would suggest to move the IntegerMapper class to the same package than the other mappers. Also maybe add a tests for the IntegerMapper methods. http://gerrit.ovirt.org/#/c/23140/9/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/utils/IntegerMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/utils/IntegerMapper.java: Line 1: package org.ovirt.engine.api.restapi.utils; Can you move this class to the package containing the other mappers? It is org.ovirt.engine.api.restapi.types. Line 2: Line 3: /** Line 4: * Utility class containing mappings between various integer representations of special values Line 5: * which differ between REST API and the backend. Line 10: * Facilitates conversion between integer that uses null to represent the default value on the backend Line 11: * to integers that use -1 for default on the rest Line 12: */ Line 13: public static Integer mapNullToMinusOne(Integer backendValue) { Line 14: if (backendValue == null) I think that we have a checkstyle rule that mandates use of curly brackets here, even if they aren't needed (I we don't have that rule we should add it). Line 15: return -1; Line 16: return backendValue; Line 17: } Line 18: -- To view, visit http://gerrit.ovirt.org/23140 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I53eaecc790d8805f55417f7eb74095001a325140 Gerrit-PatchSet: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Martin Betak <mbe...@redhat.com> 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
