-
Notifications
You must be signed in to change notification settings - Fork 80
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
Suggest obvious fix when action id includes type #1258
Conversation
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.
Looks pretty good.
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.
IMO worth adding an line in the changelog. We weren't consistent with mentioning error message changes like this leading up to the 4.0 release since we had made a ton of error message tweaks, but I like including these small improvements.
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.
Looks good overall.
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Signed-off-by: Adrian Palacios <accorell@amazon.com>
Description of changes
This PR extends the validation of action IDs so that we detect cases in which the user may have mistakenly included the entity type (i.e.,
"Action::"
) in the action ID. In that case, we don't suggest another action ID like we used to, but instead draw attention to the fact that the action ID was defined with the entity type in its name. This avoids misleading the user when the fix is much simpler.Resolves #166
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy
.I confirm that this PR (choose one, and delete the other options):
I confirm that
cedar-spec
(choose one, and delete the other options):I confirm that
docs.cedarpolicy.com
(choose one, and delete the other options):