RFC 48 implementation #1316
RFC 48 implementation #1316
Conversation
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
I did do it but realized that it wasn't in the RFC. |
fair, but empty annotation came about after the RFC was written, so I'd assume the intent of the RFC was to support annotations in schema as they exist for policies |
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
| #[cfg_attr(feature = "wasm", derive(tsify::Tsify))] | ||
| #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))] |
There was a problem hiding this comment.
Before this PR, the EST Annotations type was just #[cfg_attr(feature = "wasm", tsify(type = "Record<string, string>"))]. This PR would seem to include the .val and .loc fields in the Wasm type. Is that a breaking change? I don't think it makes sense to have .loc fields allowed in the EST. Maybe the tsify type for Annotation here should just be string, and we figure out how to let Wasm construct Annotation from just a string?
There was a problem hiding this comment.
I had some similar concerns. More generally, I just don't fully understand how this PR affects the wasm api, so I can't assert that we're not breaking it or exposing new details we'd rather not stablize
There was a problem hiding this comment.
It seems that Annotation is still a type alias to SmolStr in the generated TS files.
| #[serde(default)] | ||
| #[serde(skip_serializing_if = "BTreeMap::is_empty")] | ||
| #[serde(with = "::serde_with::rust::maps_duplicate_key_is_error")] | ||
| #[cfg_attr(feature = "wasm", tsify(type = "Record<string, Annotation>"))] |
There was a problem hiding this comment.
maybe my comment above about locs in the EST should have been here. I didn't realize there were two Annotations types now
There was a problem hiding this comment.
(but it looks like there's just the one Annotation type containing the loc, so my comment still applies)
Co-authored-by: Craig Disselkoen <cdiss@amazon.com>
| #[cfg_attr(feature = "wasm", derive(tsify::Tsify))] | ||
| #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))] |
There was a problem hiding this comment.
I had some similar concerns. More generally, I just don't fully understand how this PR affects the wasm api, so I can't assert that we're not breaking it or exposing new details we'd rather not stablize
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Signed-off-by: Shaobo He <shaobohe@amazon.com>
Description of changes
This PR implements RFC 48 (schema annotations).
Issue #, if available
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy(e.g., addition of a new API).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):cedar-docs. PRs should be targeted at astaging-X.Ybranch, notmain.)