search

plantbreeding / BrAPI

Repository for version control of the BrAPI specifications
https://brapi.org
MIT License
57 stars 32 forks source link

Two declaration of variableBaseClass #409

Closed cpommier closed 4 years ago

cpommier commented 4 years ago

https://github.com/plantbreeding/API/blob/eae9433880153c7d0f87509126a9fd72800b9bec/Specification/BrAPI-Germplasm/Germplasm_Attributes/Schemas/VariableBaseClass.yaml#L1

AND here : API/Specification/BrAPI-Phenotyping/ObservationVariables/Schemas/VariableBaseClass.yaml

Should they be moved to BrAPI core ?

BrapiCoordinatorSelby commented 4 years ago

I went back and forth on this for a while. One of the reasons for splitting up the BrAPI Modules was so that they could be updated independently. This means there should be no shared classes from one module to another. There are a set of "global" classes under API/Specification/Components which contains things like Metadata, GeoJSON and 404NotFound. These classes are automatically included in every module. VariableBaseClass is somewhere in the middle, it is used in two of the modules, but it doesn't quite feel like a "global" class. The same applies for the Trait, Method, and Scale classes.

Based on this issue I have moved all 4 classes to API/Specification/Components so they can be shared across the two modules and there is reduced risk of the copies getting out of sync.