-
Notifications
You must be signed in to change notification settings - Fork 27
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
feat(cli): Use additional permissions with config-sync command #2822
feat(cli): Use additional permissions with config-sync command #2822
Conversation
🦋 Changeset detectedLatest commit: 5f45a61 The changes in this PR will be included in the next version bump. This PR includes changesets to release 35 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Deploy preview for merchant-center-application-kit ready! ✅ Preview Built with commit 5a7680f. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also extend the tests for the CLI config diffs to include additional permissions and make sure the diffing logic still works.
Thanks, otherwise looks pretty straight forward 👌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, much better now.
Only a few couple of things to tackle and then we should be good to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯 Awesome, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👌
* feat(config): include additional permissions when using config-sync * refactor(cli): validate additional oathScope * refactor(cli): implement getPermissions function, update tests * refactor(cli): move getPermission function to transformer.ts * refactor(cli): add validation for additional permission name * refactor(cli): show duplicated permission name in error thrown
* feat(config): include additional permissions when using config-sync * refactor(cli): validate additional oathScope * refactor(cli): implement getPermissions function, update tests * refactor(cli): move getPermission function to transformer.ts * refactor(cli): add validation for additional permission name * refactor(cli): show duplicated permission name in error thrown
* refactor(config): define new field additionalOAuthScopes in app config * docs: improvements * docs: code review improvements * refactor(config): include additional oauth scopes in oidc flow (#2801) * refactor: make a separate PR with docs changes * feat(cli): Use additional permissions with config-sync command (#2822) * feat(config): include additional permissions when using config-sync * refactor(cli): validate additional oathScope * refactor(cli): implement getPermissions function, update tests * refactor(cli): move getPermission function to transformer.ts * refactor(cli): add validation for additional permission name * refactor(cli): show duplicated permission name in error thrown * refactor: update permission group name regex * test: fix snapshots * chore: alphabetic not alphanumeric * chore: update changeset * refactor: at least one additional oauth scope defined * test: typo Co-authored-by: Yemitan Isaiah Olurotimi <yemitanisaiah@gmail.com>
Summary
Update the CLI to use additional permissions