Skip to content

Recommendation20 - updates following issue #9#14

Open
rgenoulaz wants to merge 2 commits intoedi3:developfrom
rgenoulaz:recommendation20-first-rework
Open

Recommendation20 - updates following issue #9#14
rgenoulaz wants to merge 2 commits intoedi3:developfrom
rgenoulaz:recommendation20-first-rework

Conversation

@rgenoulaz
Copy link
Collaborator

FIrst step : Updated according to elements agreed in issue #9:

  • Change the name of the resource to make it a plural : unitsOfMeasure.
  • Update all parameters to camelCase
  • Change the name of one parameter that had a slash in the middle, Update description to contain the old name as information.
  • Add the descriptions based on the documentation from UN/CEFACT
  • Remove Status parameter/property
  • Change commonCode parameter type from query to path

Updated accordoing to elements agreed in usse edi3#9:

- Change the name of the resource to make it a plural : unitsOfMeasure.
-  Update all parameters to camelCase
- Change the name of one parameter that had a slash in the middle, Update description to contain the old name as information.
- Add the descriptions based on the documentation from UN/CEFACT
- Remove Status parameter/property
- Change commonCode parameter type from query to path
@rgenoulaz rgenoulaz changed the title Update swagger.yaml Recommendation20 - updates following issue #9 May 20, 2019
@kshychko
Copy link
Contributor

kshychko commented May 20, 2019

@rgenoulaz I was expecting keeping the different paths means keeping the different resources. So GET quantityCategory/{commonCode} will return a quantity category object and GET unitsOfMeasure/{commonCode} will return a unit of measure object. These two contexts might share the common properties, but I though they would be defined separately. @cmsdroff what are your thoughts on this?

@kshychko kshychko requested review from cmsdroff and kshychko May 20, 2019 18:03
@rgenoulaz
Copy link
Collaborator Author

@kshychko oh ok you want to keep :
/quantityCategory/{commonCode}
/quantityCategory
/unitsOfMeasure/{commonCode}
/unitsOfMeasure

But you want the two first ones to return a QuantityCategory object, event if it's a duplicate of unitsOfMeasure ?

If so, then why not ;)

@kshychko
Copy link
Contributor

@rgenoulaz , I think they should be different objects, as they represent data from different data sources, that's my only concern. @cmsdroff , what do you think on this?

@cmsdroff
Copy link
Contributor

cmsdroff commented Jun 9, 2019

@rgenoulaz agree with @kshychko, at this point we should keep the quantityCategory object, looking through the background of this the fields are described once in the pdf, but each annex only refers to the fields that it needs (most overlap though) and they are in different orders, which makes little sense either. International trade uses the Annex II/III only, the other must be for scientific usage, so should treat as separate objects as this is what is happening in excel today.

Suggest we add the quantityCategory object back in, merge the PR and close this ticket @rgenoulaz if you could paste it back in, I'll merge?

Maybe one for the code group in UN/CEFACT to consider is could they merge the dataset.. but outside of scope for what we are working, reality with this type of dataset is changes would be really occasional!

adding back QuantityCategory per edi3#14 (comment)
also re-using some parameters and responses definitions
@kshychko
Copy link
Contributor

@rgenoulaz I made the changes in your PR, hope you don't mind.
@cmsdroff could you please provide description for groupNumber and groupId properties and query parameters.
I moved some parameters and response definitions to components section so we can re-use them.

@kshychko kshychko removed their request for review July 28, 2019 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants