-
Notifications
You must be signed in to change notification settings - Fork 90
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
Add methods to get number of policies and templates in a PolicySet #1180
Conversation
…cy set has cedar-policy#1179 Signed-off-by: Juan Vicente Garcia Orozco <juano@amazon.com>
Signed-off-by: Juan Vicente Garcia Orozco <juano@amazon.com>
Signed-off-by: Juan Vicente Garcia Orozco <juano@amazon.com>
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 great, two nits
cedar-policy/CHANGELOG.md
Outdated
@@ -11,6 +11,7 @@ Starting with version 3.2.4, changes marked with a star (*) are _language breaki | |||
|
|||
## [Unreleased] | |||
Cedar Language Version: TBD | |||
* Add convenience methods to see how many policies and templates a policy set has (#1179). |
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.
nit: can we put this in the Added
section (three lines below)?
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.
Sure!
cedar-policy/src/api.rs
Outdated
@@ -2057,6 +2057,18 @@ impl PolicySet { | |||
self.ast.is_empty() | |||
} | |||
|
|||
/// Returns the number of `Template`s in the `PolicySet`. |
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.
/// Returns the number of `Template`s in the `PolicySet`. | |
/// Returns the number of `Policy`s in the `PolicySet`. |
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.
Ah, copy paste mishap, let me fix this.
Signed-off-by: Juan Vicente Garcia Orozco <juano@amazon.com>
One of the checks failed, is this a known issue? I'd probably need a bit of help to fix it if needed. |
It's a problem unrelated to the change in this PR. Given that, happy to merge this now -- looks like we have 2 approvals and the CI failures are unrelated. |
…1180) Signed-off-by: Juan Vicente Garcia Orozco <juano@amazon.com> Co-authored-by: Juan Vicente Garcia Orozco <juano@amazon.com>
…1180) Signed-off-by: Juan Vicente Garcia Orozco <juano@amazon.com> Co-authored-by: Juan Vicente Garcia Orozco <juano@amazon.com>
…1180) Signed-off-by: Juan Vicente Garcia Orozco <juano@amazon.com> Co-authored-by: Juan Vicente Garcia Orozco <juano@amazon.com>
Description of changes
Add methods to get number of policies and templates in a PolicySet.
Regarding unit testing, I decided to just reuse this API in previous places where the size of existing iterators are used, it feels like more effort would feel silly here.
Issue #, if available
#1179
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy
(e.g., changes to the signature of an existing API).cedar-policy
(e.g., addition of a new API).cedar-policy
.cedar-policy-core
,cedar-validator
, etc.)I confirm that this PR (choose one, and delete the other options):
I confirm that
cedar-spec
(choose one, and delete the other options):cedar-spec
, and how you have tested that your updates are correct.)cedar-spec
. (Post your PR anyways, and we'll discuss in the comments.)