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

DEVX-404 adding a docblock in the clientcredential #319

Merged
merged 4 commits into from
Jul 10, 2024
Merged

Conversation

barbara79
Copy link
Contributor

@barbara79 barbara79 commented Jul 4, 2024

Copy link

gitstream-cm bot commented Jul 4, 2024

🥷 Code experts: jenschude

jenschude has most 🧠 knowledge in the files.

See details

languages/php/src/main/kotlin/io/vrap/codegen/languages/php/base/PhpBaseFileProducer.kt

Knowledge based on git-blame:
jenschude: 99%

To learn more about /:\ gitStream - Visit our Docs

@barbara79 barbara79 requested review from lojzatran, jenschude, ajimae, evansinho and a team July 4, 2024 06:17
Copy link

@lojzatran lojzatran left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I'm wondering if this is an error of the SDK or of the API 🤔 From the linked issue it seems like an error of the API that you cannot provide scope when there is only 1 scope. If so, this should be updated also in the documentations: https://docs.commercetools.com/api/scopes. Please clarify 🙏

@jenschude
Copy link
Contributor

jenschude commented Jul 4, 2024

LGTM 👍

I'm wondering if this is an error of the SDK or of the API 🤔 From the linked issue it seems like an error of the API that you cannot provide scope when there is only 1 scope. If so, this should be updated also in the documentations: docs.commercetools.com/api/scopes. Please clarify 🙏

It's actually a bad practice to use an oauth client with multiple scopes and request a token with a fraction of them. It's always better to create an oauth client with the scopes needed and than just omit the scopes in the oauth request.

…se/PhpBaseFileProducer.kt

Co-authored-by: Jens Schulze <[email protected]>
@barbara79
Copy link
Contributor Author

LGTM 👍
I'm wondering if this is an error of the SDK or of the API 🤔 From the linked issue it seems like an error of the API that you cannot provide scope when there is only 1 scope. If so, this should be updated also in the documentations: docs.commercetools.com/api/scopes. Please clarify 🙏

It's actually a bad practice to use an oauth client with multiple scopes and request a token with a fraction of them. It's always better to create an oauth client with the scopes needed and than just omit the scopes in the oauth request.

Thanks for the clarification @jenschude !

@jenschude jenschude merged commit 2664039 into main Jul 10, 2024
6 checks passed
@jenschude jenschude deleted the php-easy-fix branch July 10, 2024 08:56
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.

3 participants