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 divide-by-zero panic messages #144

Merged
merged 1 commit into from
Apr 15, 2020
Merged

Conversation

tech6hutch
Copy link
Contributor

This brings it more in line with std's numbers, and lets the user actually know why it panicked.

@tech6hutch
Copy link
Contributor Author

I hope I targeted the right branch, by the way. I was using the version on cargo, v0.2.6, when I noticed the lack of panic messages.

Copy link
Member

@cuviper cuviper left a comment

Choose a reason for hiding this comment

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

Thanks for this cleanup!

src/bigint.rs Outdated Show resolved Hide resolved
src/biguint.rs Outdated Show resolved Hide resolved
src/biguint.rs Outdated Show resolved Hide resolved
@cuviper
Copy link
Member

cuviper commented Apr 14, 2020

I hope I targeted the right branch, by the way. I was using the version on cargo, v0.2.6, when I noticed the lack of panic messages.

The master branch is good -- I hope to wrap up 0.3 "soon". 🤞

I've made a few bugfixes on 0.2 in the meantime, but I don't think I'd bother just for a diagnostic.

src/bigint.rs Outdated Show resolved Hide resolved
@tech6hutch tech6hutch requested a review from cuviper April 15, 2020 21:07
@tech6hutch
Copy link
Contributor Author

I did these changes from the GitHub editor, which is why there are so many commits. I assume you squash and merge anyway?

@cuviper
Copy link
Member

cuviper commented Apr 15, 2020

I assume you squash and merge anyway?

I use a bot that does a plain merge, without squashing. I prefer to allow that PRs may have a series of meaningful commits, but in this case the separate commits are not helpful. So it would be great if you could squash this branch, or I can quickly do so myself.

@tech6hutch
Copy link
Contributor Author

So it would be great if you could squash this branch, or I can quickly do so myself.

It'd be easier for you to do it, please. Thank you.

This brings it more in line with std's numbers, and lets the user actually know why it panicked.
@cuviper
Copy link
Member

cuviper commented Apr 15, 2020

Thanks again!

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 15, 2020

Build succeeded:

@bors bors bot merged commit b3a21a0 into rust-num:master Apr 15, 2020
@tech6hutch tech6hutch deleted the patch-1 branch April 16, 2020 02:13
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

Successfully merging this pull request may close these issues.

2 participants