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

API Design Guidelines chapter 11: 'info' object, 'servers' object and cleanup #214

Merged
merged 8 commits into from
Jun 14, 2024

Conversation

rartych
Copy link
Collaborator

@rartych rartych commented May 24, 2024

What type of PR is this?

Add one of the following kinds:

  • correction

What this PR does / why we need it:

The definitions of 'info' and 'servers` object were added to chapter 11.
Other edits on API definition guidelines.
I propose to remove the figures as they do not add value to the text.

Which issue(s) this PR fixes:

Fixes #199
Fixes #201

Special notes for reviewers:

Info object described in the same manner as here: https://swagger.io/docs/specification/api-general-info/

Changelog input

The definitions of 'info' and 'servers` object were added to chapter 11

Additional documentation

Copy link
Collaborator

@shilpa-padgaonkar shilpa-padgaonkar left a comment

Choose a reason for hiding this comment

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

Added a small comment to be consistent. Rest /LGTM

Co-authored-by: Shilpa Padgaonkar <77152136+shilpa-padgaonkar@users.noreply.github.com>
info object comments updated
@shilpa-padgaonkar
Copy link
Collaborator

@patrice-conil , @PedroDiez and @RubenBG7 : Could you kindly review & approve this if all looks ok? we are trying to speed up reviews of at least the straightforward/simple PRs so that we are in time for the 0.4 release. Thanks in advance.

Copy link
Collaborator

@shilpa-padgaonkar shilpa-padgaonkar left a comment

Choose a reason for hiding this comment

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

/LGTM

patrice-conil
patrice-conil previously approved these changes May 28, 2024
Copy link
Collaborator

@patrice-conil patrice-conil left a comment

Choose a reason for hiding this comment

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

/LGTM

Example of servers object changed
Copy link
Collaborator

@shilpa-padgaonkar shilpa-padgaonkar left a comment

Choose a reason for hiding this comment

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

/LGTM

@shilpa-padgaonkar
Copy link
Collaborator

shilpa-padgaonkar commented May 31, 2024

@PedroDiez or @RubenBG7 Could one of you kindly approve this PR (if you find no issues) so that we can proceed to merge? Thanks in advance.

termsOfService: http://example.com/terms/
# Contact information: name, email, URL
contact:
name: API Support
Copy link
Collaborator

Choose a reason for hiding this comment

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

One doubt
which values are expected to be used in the APIs (I assume we will have to insert this information in every API so i think we could have some alignment regarding the value to be indicated to use the same approach among APIs or it is up to the WG the value to be provided).
Anycase, all of them (name, email, url) has to be indicated within the yaml, doesn`t?

name: support -> e.g. Blockchain Public Address Support
email: mail distribution list of the WG -> e.g. sp-bpa@lists.camaraproject.org
url: link to the WG site in CAMARA gtihub -> e.g. https://github.com/camaraproject/BlockchainPublicAddress

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it is assumed that the contact information will be changed by each API Provider (Operator or Aggregator) when publishing API specification as the documentation for their service - possibly it should be explained in API Design Guidelines here in chapter 11.
When yaml is hosted in CAMARA Github all contact info is in main README.md
@tanjadegroot @hdamker WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should include the information that is relevant for CAMARA in our specs, as each API provider will adapt our OAS spec to whatever template or format they use in their portals. title and version are the only required sub-sections for info section. If we decided to add the contact sub-section, I would fill it with contact details for CAMARA.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I had added such contact subsection with Camara relevant details in the common.yaml as a part of the last release and would personally prefer this format as well.

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Following the discussion in Release Management: https://wiki.camaraproject.org/display/CAM/2024-06-04+Release+WG+Minutes and #214 (comment) I have removed contact and termsOfService from the info object and added some explanation.

See the changes: 2421afb.

@tanjadegroot
Copy link
Contributor

tanjadegroot commented Jun 4, 2024 via email

@rartych
Copy link
Collaborator Author

rartych commented Jun 6, 2024

Following the discussion in Release Management: https://wiki.camaraproject.org/display/CAM/2024-06-04+Release+WG+Minutes
I have added x-camara-commonalities extension field to the info object.

Copy link
Collaborator

@PedroDiez PedroDiez left a comment

Choose a reason for hiding this comment

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

LGTM

@patrice-conil
Copy link
Collaborator

LGTM

Copy link
Collaborator

@patrice-conil patrice-conil left a comment

Choose a reason for hiding this comment

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

/LGTM

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.

LGTM

Copy link
Collaborator

@shilpa-padgaonkar shilpa-padgaonkar left a comment

Choose a reason for hiding this comment

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

/LGTM

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.

Add guidance for Info object in apis Remove basePath in servers.url as variable
7 participants