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

LibSemVer: Propogate errors so we get an error instead of crashing #25525

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ninadsachania
Copy link
Contributor

No description provided.

@nico
Copy link
Contributor

nico commented Dec 2, 2024

Thanks for the patch! It looks like these functions are in userland and just allocate fairly fall strings, so per #20405 the SemVer accessors allocating strings should just MUST() instead of release_value_but_fixme_...

I don't remember if the developer docs mention the current stance on error handling mentioned in that issue. If they don't yet, they should be updated to do so…

@ninadsachania
Copy link
Contributor Author

Thanks for the patch! It looks like these functions are in userland and just allocate fairly fall strings, so per #20405 the SemVer accessors allocating strings should just MUST() instead of release_value_but_fixme_...

I don't remember if the developer docs mention the current stance on error handling mentioned in that issue. If they don't yet, they should be updated to do so…

Ahh, that makes so much sense. I also felt like some of these refactorings are making things ugly, but I was following the FIXMEs. I'll fix the patch.

Copy link

stale bot commented Dec 28, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

@stale stale bot added the stale label Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants