Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding global tags element #227

Merged
merged 3 commits into from
Nov 6, 2023
Merged

Adding global tags element #227

merged 3 commits into from
Nov 6, 2023

Conversation

rartych
Copy link
Contributor

@rartych rartych commented Oct 17, 2023

What type of PR is this?

  • correction

What this PR does / why we need it:

Operation tags should be defined in a top-level tags element.

Which issue(s) this PR fixes:

Fixes #223

Special notes for reviewers:

Description field is rendered by Swagger this way:
QoD_tags_in_Swagger

Changelog input

Global tags element added

Additional documentation

Adding global tags array
Copy link
Collaborator

@hdamker hdamker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small correction necessary to make linting happy ;-)

Trailing spaces removed

Co-authored-by: Herbert Damker <[email protected]>
Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use same format for both tags, either both sessions and profiles in lowecase or uppercase. Tags per operation have to be adjusted accordingly

@hdamker
Copy link
Collaborator

hdamker commented Oct 18, 2023

@jlurien

We should use same format for both tags, either both sessions and profiles in lowecase or uppercase. Tags per operation have to be adjusted accordingly

I looked over the documentation parts, and there we have also the same difference, it's more or less consequently "QoS session" and "QoS Profile". But I propose to not extend the scope of this PR to a cleanup of the documentation parts, and do it here only for the tags.

I would tend to lowercase "QoS session" and "QoS profile", but that's just my view. Any other views?

@eric-murray
Copy link
Collaborator

I would tend to lowercase "QoS session" and "QoS profile", but that's just my view. Any other views?

My preference is Title Case, but this is a style point and really one for Commonalities and the design guidelines., whose example tags are unfortunately ambiguous on this point.

@rartych
Copy link
Contributor Author

rartych commented Oct 19, 2023

I do no think we need exact guidelines for tags style/format, as tags are helper properties to clearly describe and render OAS and are useful with large API specifications.
I have aligned to the current style of tags, with the reasoning that while sessions are dynamic and created on request, QoD Profiles were initially predefined and static.
But I agree that having just "QoS session" and "QoS profile" is good solution.
Should the tags and their descriptions be updated in this PR?
Or together with separate issue for unification of the style in all the documentation?

@eric-murray
Copy link
Collaborator

If we want a common "look and feel" between CAMARA APIs, then we need an agreement in Commonalities on tag format style.

QoS sessions --> QoS Sessions
Copy link
Collaborator

@RandyLevensalor RandyLevensalor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thanks for adding this.

@hdamker
Copy link
Collaborator

hdamker commented Nov 6, 2023

Good to go. Thanks @rartych !

@hdamker hdamker merged commit b4e93ca into camaraproject:main Nov 6, 2023
2 checks passed
@rartych rartych deleted the patch-1 branch December 7, 2023 12:07
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.

Operation tags in global tags element
5 participants