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 try_ variants to some methods, make existing methods panic #284

Open
joshlf opened this issue Aug 25, 2023 · 2 comments
Open

Add try_ variants to some methods, make existing methods panic #284

joshlf opened this issue Aug 25, 2023 · 2 comments
Labels
compatibility-breaking Changes that are (likely to be) breaking

Comments

@joshlf
Copy link
Member

joshlf commented Aug 25, 2023

Currently, we have methods like write_to and write_to_prefix return Option<()> in order to signal success or failure ("failure" here means that the provided bytes: &mut [u8] wasn't long enough). This breaks with the common Rust pattern, which would have these simply panic in this case.

In my opinion, it's preferable to avoid panicking when possible, so it's good to provide methods with this behavior. However, we could get the best of both worlds and be more consistent with Rust style by having methods like write_to panic, and introducing new methods like try_write_to which return Option<()>.

See also #188

@itsyaasir
Copy link
Contributor

itsyaasir commented Aug 27, 2023

@joshlf I would like to take a look at this.
Thank you!

@joshlf
Copy link
Member Author

joshlf commented Aug 28, 2023

Hey @itsyaasir, thanks for volunteering! This isn't actually something I'm ready to commit to just yet - I've just written it down so we don't forget and can decide later whether we definitely want to do it. If you want to see what other issues we have that are ready for outside contribution, take a look at our help wanted Extra attention is needed label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility-breaking Changes that are (likely to be) breaking
Projects
None yet
Development

No branches or pull requests

2 participants