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.

Sign up for GitHub

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

Jump to bottom

PR #182 split - info.description template review (part 3 of 3) #214

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

PR #182 split - info.description template review (part 3 of 3) #214

wants to merge 4 commits into from

Conversation

jpengar
Copy link
Collaborator

@jpengar jpengar commented Oct 10, 2024 •
edited
Loading

What type of PR is this?

  • documentation

What this PR does / why we need it:

As discussed by the team in 11/09 WG call, in order to move things forward, we are going to split the content of PR #182 into three different PRs:

The WG concluded that it would be much easier to deal with if we separated the proposed changes in #182 into different PRs.

Which issue(s) this PR fixes:

Fixes #178

Special notes for reviewers:

Further context in #182

CC @chrishowell

Changelog input

Improved document writing

Additional documentation

N/A

This was referenced Oct 10, 2024
Merged
Merged
Closed
Open
@jpengar
Copy link
Collaborator Author

jpengar commented Oct 22, 2024

@chrishowell's suggestions are fine with me. But in this case, as we are updating the info.description template, which affects all API specs, I think the final agreed text will need to be validated by the TSC for final approval.

@jpengar
Copy link
Collaborator Author

jpengar commented Oct 22, 2024

I invite WG participants to review and comment. Specifically related issue owners. @RandyLevensalor
CC @AxelNennker @sebdewet

@jpengar
Copy link
Collaborator Author

jpengar commented Nov 4, 2024

Any comments on this PR? If there are no comments, I would propose to keep the current agreed info.description template and discard these changes.

RandyLevensalor
RandyLevensalor suggested changes Nov 4, 2024
View reviewed changes
Copy link

@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.

This is an improvement. Just a few minor comments.

documentation/CAMARA-API-access-and-user-consent.md Outdated Show resolved Hide resolved
@jpengar
Copy link
Collaborator Author

jpengar commented Nov 6, 2024

@camaraproject/tsc-participants As we are updating the info.description template, which affects all API specs, I think the final agreed text will need to be validated by the TSC for final approval as done in the past. Am I right?

tanjadegroot
tanjadegroot reviewed Nov 6, 2024
View reviewed changes
documentation/CAMARA-API-access-and-user-consent.md Outdated

Which specific authorization flows are to be used will be determined during onboarding process, happening between the API Client and the Telco Operator exposing the API, taking into account the declared purpose for accessing the API, while also being subject to the prevailing legal framework dictated by local legislation.
The specific authorization flows to be used will be determined during the onboarding process, happening between the API client and the operator exposing the API, taking into account the declared purpose for accessing the API, whilst also being subject to the prevailing legal framework dictated by local legislation.
Copy link

Choose a reason for hiding this comment

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

The type of auth flow is established during the onboarding process, and hence outside the API run-time usage, correct ? we may want to be more explicit on that. point.

so I suggest to

  • replace "determined" by "agreed upon"
  • replace "API client" by "owner of the application consuming the API and the owner of the API exposure platform"

Copy link
Collaborator Author

@jpengar jpengar Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hopefully fixed in 00bac93

documentation/CAMARA-API-access-and-user-consent.md Outdated

```
### Authorization and authentication

The "Camara Security and Interoperability Profile" provides details on how a client requests an access token. Please refer to Identify and Consent Management (https://github.com/camaraproject/IdentityAndConsentManagement/) for the released version of the Profile.
The "Camara Security and Interoperability Profile" provides details on how a client requests an access token.
Copy link

Choose a reason for hiding this comment

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

replace "client" by "API consumer"

Can this text be more explicit and say "how an API consumer SHALL request an access token" ?

Copy link
Collaborator Author

@jpengar jpengar Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hopefully fixed in 00bac93. I've restored the link to the ICM WG, which I missed. I prefer to keep it as well. Not sure if It was a change originally requested in #182.

RandyLevensalor
RandyLevensalor approved these changes Nov 12, 2024
View reviewed changes
Copy link

@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.

This is much more clear. Thanks for making the improvements.

@AxelNennker
Copy link
Collaborator

@tanjadegroot please review again

@rartych rartych mentioned this pull request Nov 20, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RandyLevensalor RandyLevensalor RandyLevensalor approved these changes

@AxelNennker AxelNennker Awaiting requested review from AxelNennker AxelNennker is a code owner

@sebdewet sebdewet Awaiting requested review from sebdewet sebdewet is a code owner

@chrishowell chrishowell Awaiting requested review from chrishowell

@tanjadegroot tanjadegroot Awaiting requested review from tanjadegroot

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The wording in the Mandatory template for info.description is unclear
4 participants
@jpengar @AxelNennker @RandyLevensalor @tanjadegroot