Juan Hernandez has posted comments on this change. Change subject: core,webadmin: Remove of storage pool type ......................................................................
Patch Set 8: (5 comments) If I understand correctly we intend to remove completely the "storage_type" in the future. If so please add comments or the @Deprecated annotation in the relevant places, for example add @Deprecated to the org.ovirt.engine.api.model.StorageType class. http://gerrit.ovirt.org/#/c/23402/8/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd: Line 1121: Line 1122: <xs:complexType name="DataCenter"> Line 1123: <xs:complexContent> Line 1124: <xs:extension base="BaseResource"> Line 1125: <xs:sequence> Can you add here a comment explaining that this is deprecated and will be removed in the future? Please add comments like this in all the places that should be changed when in the future we actually remove it. Line 1126: <xs:element name="storage_type" type="xs:string" minOccurs="0" /> Line 1127: <xs:element name="local" type="xs:boolean" minOccurs="0" /> Line 1128: <xs:element name="storage_format" type="xs:string" minOccurs="0" /> Line 1129: <xs:element name="version" type="Version" minOccurs="0" /> http://gerrit.ovirt.org/#/c/23402/8/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml File backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml: Line 1706: optionalArguments: Line 1707: datacenter.name: xs:string Line 1708: datacenter.description: xs:string Line 1709: datacenter.comment: xs:string Line 1710: datacenter.storage_type: xs:string Add --DEPRECATD here, that way it will be documented in the RSDL document: datacenter.storage_type--DEPRECATED: xs:string Line 1711: datacenter.local: xs:boolean Line 1712: datacenter.version.major: xs:int Line 1713: datacenter.version.minor: xs:int Line 1714: datacenter.description: xs:string Line 1761: parameterType: DataCenter Line 1762: signatures: Line 1763: - mandatoryArguments: {datacenter.name: 'xs:string', datacenter.version.major: 'xs:int', datacenter.version.minor: 'xs:int'} Line 1764: optionalArguments: Line 1765: datacenter.storage_type: xs:string Use --DEPRECATED here as well. Line 1766: datacenter.local: xs:boolean Line 1767: datacenter.description: xs:string Line 1768: datacenter.comment: xs:string Line 1769: datacenter.storage_format: xs:string http://gerrit.ovirt.org/#/c/23402/8/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/DataCenterMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/DataCenterMapper.java: Line 57 Line 58 Line 59 Line 60 Line 61 We should populate the "storage_type" element, otherwise existing clients that use it can break. http://gerrit.ovirt.org/#/c/23402/8/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/DataCenterMapperTest.java File backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/DataCenterMapperTest.java: Line 18: } Line 19: Line 20: @Override Line 21: protected DataCenter postPopulate(DataCenter model) { Line 22: model.setLocal(new Random().nextBoolean()); This shouldn't be needed. The model is already randomly populated in the "populate" method. Enum types need this special treatment, but not booleans. Line 23: model.setStorageFormat(MappingTestHelper.shuffle(StorageFormat.class).value()); Line 24: return model; Line 25: } Line 26: -- To view, visit http://gerrit.ovirt.org/23402 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If29a4ecb9aa284b57e9f5218ca50cf4287452e3e Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tal Nisan <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: Vered Volansky <[email protected]> Gerrit-Reviewer: Yaniv Dary <[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
