-
Notifications
You must be signed in to change notification settings - Fork 0
W-22034567: remove startDate, endDate from license provision command and align json definition file fields with cli flags and enforce required flags #8
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is a very minor discussion point.
@acardel
Do we want to try to "align" the field names between the CLI and the API?
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.
Agree, specially in the case of
licensewhich could refer to other types of licenses on the platform.We should match
licensetopermissionSetLicenseand namespace to
namespacePrefixWhat do you say @ukanoja-sf?
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.
The one thing we should probably be mindful of is not locking the sf cli command into PSL specific naming, since it is possible that this could be used for non-PSL related provisioning in the future.
So realistically, the only thing we might want to converge on is namespace.
tbh, it looks like the existing packaging code uses "namespace" as the parameter name.
So if we want to align to that, we'd be changing the API not the CLI.
Or we just let it go. Perhaps we ask Rashmi the importance of the delivery timing.
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.
I think after discussion with platform team we have decided to use just
licenseand our design doc also mentions the same params. I prefer current naming and also not sure if we update now, then we might need another design review with platform team. If want to change, then I will prefer to update it in api request. @trey-sf, @acardel let me know if you prefer to discuss it over call (or tomorrow's standup) and we can quickly resolve it.https://salesforce.quip.com/ysrEAiCWQfak#temp:C:YWc448cff01faa74dc0b48f1cf93
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.
saw @trey-sf's comment after posting my comment, I will bring this up in tomorrow's stand up and we can close this in parking lot.
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.
Cool. If that's the case I lean towards letting it go. I like the API to be PSL specific. If a user goes straight to the endpoint they will be hitting the PermissionSetLicense resource.
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.
I'd definitely leave PermissionSetLicense as-is.
It's whether we rename "namespacePrefix" to just "namespace" in the API.
Let's discuss in PL.