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

Add APIs using Result with thiserror or anyhow crates instead of panic #16

Closed
tats-u opened this issue Oct 1, 2024 · 2 comments
Closed

Comments

@tats-u
Copy link

tats-u commented Oct 1, 2024

https://github.com/search?q=repo%3APSeitz%2Fwana_kana_rust%20panic&type=code

This library can panic and discourages other software from adopting this library.

#13

e.g. https://github.com/orgs/meilisearch/discussions/532#discussioncomment-9999921

https://docs.rs/thiserror/latest/thiserror/ and https://docs.rs/anyhow/latest/anyhow/ are widely used to handle errors without panic.

New APIs like to_kana_safe() and to_romaji_safe() will be appreciated.

@PSeitz
Copy link
Owner

PSeitz commented Oct 1, 2024

The library should never panic. Error's don't make sense imo, since it works on a best effort basis.

Here's how I addressed the issue:

  1. Fixed the original issue
  2. Added some proptests with random and semi random input to check for panics
  3. Checked the code for other potential panics

@PSeitz PSeitz closed this as completed Oct 1, 2024
@tats-u
Copy link
Author

tats-u commented Oct 2, 2024

Oh snap, looks like all of them are test code. Sorry to bother you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants