Martin Betak has posted comments on this change. Change subject: restapi: Enable configuration of Serial Number Policy ......................................................................
Patch Set 2: (5 comments) http://gerrit.ovirt.org/#/c/25100/2/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/SerialNumberMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/SerialNumberMapper.java: Line 15: template.setPolicy(policy.value()); Line 16: } Line 17: if (policy == SerialNumberPolicy.CUSTOM) { Line 18: template.setValue(entity.getCustomSerialNumber()); Line 19: } > The API should not check for a particular value, it should render whatever Done Line 20: Line 21: return template; Line 22: } Line 23: Line 26: SerialNumberPolicy serialNumberPolicy = SerialNumberPolicy.fromValue(serialNumber.getPolicy()); Line 27: entity.setSerialNumberPolicy(map(serialNumberPolicy, null)); Line 28: entity.setCustomSerialNumber(serialNumber.getValue()); Line 29: Line 30: return entity; > Please remove the @Mapping annotation and rename the method to "copySerialN Done Line 31: } Line 32: Line 33: @Mapping(from = SerialNumberPolicy.class, to = org.ovirt.engine.core.common.businessentities.SerialNumberPolicy.class) Line 34: public static org.ovirt.engine.core.common.businessentities.SerialNumberPolicy map(SerialNumberPolicy serialNumberPolicy, org.ovirt.engine.core.common.businessentities.SerialNumberPolicy template) { Line 40: return org.ovirt.engine.core.common.businessentities.SerialNumberPolicy.HOST_ID; Line 41: case VM: Line 42: return org.ovirt.engine.core.common.businessentities.SerialNumberPolicy.VM_ID; Line 43: case CUSTOM: Line 44: return org.ovirt.engine.core.common.businessentities.SerialNumberPolicy.CUSTOM; > In the rest of the API code we use the following style for switch statement Done Line 45: } Line 46: return null; Line 47: } Line 48: Line 56: return SerialNumberPolicy.HOST; Line 57: case VM_ID: Line 58: return SerialNumberPolicy.VM; Line 59: case CUSTOM: Line 60: return SerialNumberPolicy.CUSTOM; > Same about the style of the switch statement. Done Line 61: } Line 62: return null; Line 63: } http://gerrit.ovirt.org/#/c/25100/2/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java: Line 176: // numbering is generated in the backend, hence even if user specified version number, we ignore it. Line 177: } Line 178: Line 179: if (model.isSetSerialNumber()) { Line 180: SerialNumberMapper.map(model.getSerialNumber(), entity); > This usage of the "map" method is a bit unusual. You are using it only for Done Line 181: } Line 182: Line 183: return entity; Line 184: } -- To view, visit http://gerrit.ovirt.org/25100 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c678c70a647e9df8954374424e9a3ff0938b45a Gerrit-PatchSet: 2 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: 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
