camaraproject / QualityOnDemand

Repository to describe, develop, document and test the QualityOnDemand API family
https://wiki.camaraproject.org/x/zwOeAQ
Apache License 2.0
38 stars 60 forks source link

doc/update-spec-doc-issue-144 #151

Closed jlurien closed 1 year ago

jlurien commented 1 year ago

What type of PR is this?

What this PR does / why we need it:

Consolidate into OpenAPI spec file all relevant documentation previously separated in QoD_API.md

Legacy file is kept and marked as "To be deprecated", we can eventually decide to remove it.

Which issue(s) this PR fixes:

Fixes #144

Special notes for reviewers:

Visualizations tested with editor.swagger.com and app.redocly.com

Changelog input

CAMARA documentation is now embedded within the OAS definition, and not separate. 

Additional documentation

This section can be blank.

docs
jlurien commented 1 year ago

@hdamker No new reviews so far. Anyway, I would keep this PR open until we merge the rest of PRs to be included in v0.9, as this PR would have to be aligned and resolve likely conflicts,

RandyLevensalor commented 1 year ago

Should we have max line length of 119 characters or something similar?

The unlimited line length is easier to manage, but causes issues in some editors.

jlurien commented 1 year ago

Should we have max line length of 119 characters or something similar?

The unlimited line length is easier to manage, but causes issues in some editors.

If max line is forced to certain characters it cannot be adjusted dynamically by the viewer to the screen width, which it is convenient., while modern editors allow to wrap lines to window size if desired

jlurien commented 1 year ago

@hdamker rebased to new updated main. If #155 is merged fro v0.9 I'll have to sync again

jlurien commented 1 year ago

@hdamker Sync'ed with new master and added those 2 missing properties to examples. I think it is all aligned in the .yaml. I have kept the old .md doc with the "deprecated note", but you may consider to remove it entirely. Let me know if we should do it as part of this PR.

hdamker commented 1 year ago

@hdamker Sync'ed with new master and added those 2 missing properties to examples. I think it is all aligned in the .yaml. I have kept the old .md doc with the "deprecated note", but you may consider to remove it entirely. Let me know if we should do it as part of this PR.

Thanks a lot @jlurien. My view is that we should remove the deprecated .md documentation from the release as otherwise we would need to review it for consistency with the .yaml.

jlurien commented 1 year ago

@jlurien Thanks for the PR

Could you please address this comment as well?

We may need to add a description to QOS_STATUS_CHANGED event that only AVAILABLE or UNAVAILABLE are the applicable qosStatus values.

Added some description

jlurien commented 1 year ago

@hdamker Sync'ed with new master and added those 2 missing properties to examples. I think it is all aligned in the .yaml. I have kept the old .md doc with the "deprecated note", but you may consider to remove it entirely. Let me know if we should do it as part of this PR.

Thanks a lot @jlurien. My view is that we should remove the deprecated .md documentation from the release as otherwise we would need to review it for consistency with the .yaml.

Agree, just removed in the PR