euniversal / umei-api-specification

UMEI API specifications
Apache License 2.0
3 stars 0 forks source link

Review return codes and rename IDs #8

Closed cdmNSIDE closed 2 years ago

cdmNSIDE commented 2 years ago

…nd 201 for put, 200 for patch).

Rename id in path. More explicit description of return codes. Rename id in components.

narve commented 2 years ago

I don't have a strong opinion on renaming some parameters and expanding on the documentation, although I feel that it does not add great value. However, one question: Why do we add all these media types? Do we require the FMO to support text/html, text/csv etc? That would be quite a heavy requirement. Furthermore those media types are not fully standardized, so without some extra documentation it might lead to incompatible implementations.

cdmNSIDE commented 2 years ago

Why do we add all these media types? Do we require the FMO to support text/html, text/csv etc? That would be quite a heavy requirement. Furthermore those media types are not fully standardized, so without some extra documentation it might lead to incompatible implementations.

Well, this media types were already included in your code, I did not change that... I would prefer to keep only the json format everywhere

cdmNSIDE commented 2 years ago

I don't have a strong opinion on renaming some parameters and expanding on the documentation, although I feel that it does not add great value.

Well for me the API is much more readable with those new names. Several IDs are used in the API, and having different names for them reduces the risk of misunderstanding.

cdmNSIDE commented 2 years ago

Any comment on the changes in HTTP return codes, and their description ?

narve commented 2 years ago

Why do we add all these media types? Do we require the FMO to support text/html, text/csv etc? That would be quite a heavy requirement. Furthermore those media types are not fully standardized, so without some extra documentation it might lead to incompatible implementations.

Well, this media types were already included in your code, I did not change that... I would prefer to keep only the json format everywhere

I also prefer JSON format everywhere. Although the NODES API supports CSV and HTML, it should not be part of the UEMI standard (at the moment).

However, these media types were introduced by your commit, as you can see here:

https://github.com/euniversal/umei-api-specification/commit/094216557e59aedbc0a88dee51ffdb4c6d388483.patch

narve commented 2 years ago

So yes, I agree on the return

Any comment on the changes in HTTP return codes, and their description ?

Those seems fine, no objections there.

narve commented 2 years ago

Why do we add all these media types? Do we require the FMO to support text/html, text/csv etc? That would be quite a heavy requirement. Furthermore those media types are not fully standardized, so without some extra documentation it might lead to incompatible implementations.

Well, this media types were already included in your code, I did not change that... I would prefer to keep only the json format everywhere

I also prefer JSON format everywhere. Although the NODES API supports CSV and HTML, it should not be part of the UEMI standard (at the moment).

However, these media types were introduced by your commit, as you can see here:

https://github.com/euniversal/umei-api-specification/commit/094216557e59aedbc0a88dee51ffdb4c6d388483.patch

My bad, the media types were already present, like you said.