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.

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

Remove ParseErrors::errors_as_strings #543

Closed
2 tasks
aaronjeline opened this issue Dec 21, 2023 · 6 comments · Fixed by #882
Closed
2 tasks

Remove ParseErrors::errors_as_strings #543

aaronjeline opened this issue Dec 21, 2023 · 6 comments · Fixed by #882
Labels
backlog We hope to work on this in the future breaking-change This is (likely) a breaking change

Comments

@aaronjeline
Copy link
Contributor

Category

Internal refactors/changes

Describe the feature you'd like to request

It's not necessary and not public

Describe alternatives you've considered

Leave it

Additional context

No response

Is this something that you'd be interested in working on?

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change
@aaronjeline aaronjeline added the internal-improvement Refactoring, performance improvement, or other non-breaking change label Dec 21, 2023
@cdisselkoen
Copy link
Contributor

I think it might be public. It's a pub method on the ParseErrors type, which is exposed in cedar-policy (see docs.rs docs for 3.0).

It is problematic though because, as we continue to invest more in miette, it only grabs the main error message, not any of the other miette data such as error code or help string

@khieta khieta added the backlog We hope to work on this in the future label Dec 21, 2023
@john-h-kastner-aws
Copy link
Contributor

Bumping this as a breaking change to make alongside the rest of the breaking changes to the error API in #745

@john-h-kastner-aws john-h-kastner-aws added breaking-change This is (likely) a breaking change and removed internal-improvement Refactoring, performance improvement, or other non-breaking change labels May 2, 2024
@cdisselkoen
Copy link
Contributor

Yes. I believe that #800 removed the last user of this API

@john-h-kastner-aws
Copy link
Contributor

Turns out #616 and #729 added three uses of this function in the wasm crate.

@cdisselkoen
Copy link
Contributor

Those should probably be replaced by uses of the new ffi::DetailedError

@cdisselkoen
Copy link
Contributor

(That's the substitution that #800 made in the other APIs)

@john-h-kastner-aws john-h-kastner-aws linked a pull request May 17, 2024 that will close this issue
3 tasks
john-h-kastner-aws added a commit that referenced this issue May 17, 2024
Signed-off-by: John Kastner <jkastner@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog We hope to work on this in the future breaking-change This is (likely) a breaking change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants