Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Attach source code to certain errors #1351

Merged
merged 10 commits into from
Dec 5, 2024
Merged

Attach source code to certain errors #1351

merged 10 commits into from
Dec 5, 2024

Conversation

shaobo-he-aws
Copy link
Contributor

@shaobo-he-aws shaobo-he-aws commented Dec 2, 2024 •
edited
Loading

Description of changes

Issue #s, if available

#977, #1335

Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

  • A change "invisible" to users (e.g., documentation, changes to "internal" crates like cedar-policy-core, cedar-validator, etc.)

I confirm that this PR (choose one, and delete the other options):

  • Updates the "Unreleased" section of the CHANGELOG with a description of my change (required for major/minor version bumps).

I confirm that cedar-spec (choose one, and delete the other options):

  • Does not require updates because my change does not impact the Cedar formal model or DRT infrastructure.

I confirm that docs.cedarpolicy.com (choose one, and delete the other options):

  • Does not require updates because my change does not impact the Cedar language specification.

@shaobo-he-aws shaobo-he-aws linked an issue Dec 2, 2024 that may be closed by this pull request
Some Diagnostic implementations set labels without source_code #977
Closed
2 tasks
@shaobo-he-aws shaobo-he-aws marked this pull request as draft December 2, 2024 23:33
@shaobo-he-aws
cedar schema parse errors
11e5bbb 
Signed-off-by: Shaobo He <[email protected]>
@shaobo-he-aws shaobo-he-aws mentioned this pull request Dec 3, 2024
Open
2 tasks
@john-h-kastner-aws
Copy link
Contributor

Should probably also fix #948

@shaobo-he-aws shaobo-he-aws marked this pull request as ready for review December 3, 2024 19:17
@shaobo-he-aws
updated changelog
ba05696 
Signed-off-by: Shaobo He <[email protected]>
john-h-kastner-aws
john-h-kastner-aws approved these changes Dec 3, 2024
View reviewed changes
cedar-policy-core/src/parser/err.rs Outdated Show resolved Hide resolved
cdisselkoen
cdisselkoen approved these changes Dec 3, 2024
View reviewed changes
Copy link
Contributor

@cdisselkoen cdisselkoen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

cedar-policy-core/src/ast/name.rs Outdated Show resolved Hide resolved
cedar-policy-core/src/test_utils.rs Outdated Show resolved Hide resolved
cedar-policy-core/src/test_utils.rs Outdated Show resolved Hide resolved
cedar-policy-validator/src/cedar_schema/err.rs Outdated Show resolved Hide resolved
shaobo-he-aws and others added 5 commits December 3, 2024 13:19
@shaobo-he-aws
probably too much for fixing a source location?
55f4bdf 
Signed-off-by: Shaobo He <[email protected]>
@shaobo-he-aws
changed to educe
e0fec08 
Signed-off-by: Shaobo He <[email protected]>
@shaobo-he-aws @cdisselkoen
Apply suggestions from code review
0bbe8d6 
Co-authored-by: Craig Disselkoen <[email protected]>
@shaobo-he-aws
review feedback
825f41f 
Signed-off-by: Shaobo He <[email protected]>
@shaobo-he-aws
macros
a171b91 
Signed-off-by: Shaobo He <[email protected]>
cdisselkoen
cdisselkoen approved these changes Dec 5, 2024
View reviewed changes
Copy link
Contributor

@cdisselkoen cdisselkoen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

cedar-policy-core/Cargo.toml Outdated Show resolved Hide resolved
cedar-policy-core/src/ast/policy.rs Outdated Show resolved Hide resolved
shaobo-he-aws and others added 2 commits December 5, 2024 10:20
@shaobo-he-aws @cdisselkoen
Update cedar-policy-core/src/ast/policy.rs
a6e66c7 
Co-authored-by: Craig Disselkoen <[email protected]>
@shaobo-he-aws
fixes
f93e6b7 
Signed-off-by: Shaobo He <[email protected]>
@shaobo-he-aws shaobo-he-aws merged commit 2647376 into main Dec 5, 2024
19 checks passed
@shaobo-he-aws shaobo-he-aws deleted the 977-some-diagnostic-implementations-set-labels-without-source_code branch December 5, 2024 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@john-h-kastner-aws john-h-kastner-aws john-h-kastner-aws approved these changes

@cdisselkoen cdisselkoen cdisselkoen approved these changes

@adpaco-aws adpaco-aws Awaiting requested review from adpaco-aws

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Some Diagnostic implementations set labels without source_code
3 participants
@shaobo-he-aws @john-h-kastner-aws @cdisselkoen