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

Correct Ratio::is_negative and Ratio::is_positive #182

Merged
merged 1 commit into from
Apr 13, 2016

Conversation

ollie27
Copy link
Contributor

@ollie27 ollie27 commented Mar 10, 2016

Zero is not positive or negative.

This was broken in 8be7e7b.

@cuviper
Copy link
Member

cuviper commented Mar 10, 2016

Ah, good catch! Looks right -- I just want to hold this for a bit so #164 isn't disrupted...

@homu
Copy link
Contributor

homu commented Apr 13, 2016

☔ The latest upstream changes (presumably #164) made this pull request unmergeable. Please resolve the merge conflicts.

@ollie27
Copy link
Contributor Author

ollie27 commented Apr 13, 2016

Rebased.

@cuviper
Copy link
Member

cuviper commented Apr 13, 2016

Just noticed, is there a reason you used bitwise & and | instead of lazy-boolean && and ||?

Zero is not positive or negative.

This was broken in 8be7e7b.
@ollie27
Copy link
Contributor Author

ollie27 commented Apr 13, 2016

I've no idea, I've changed it to && and || anyway.

@cuviper
Copy link
Member

cuviper commented Apr 13, 2016

Thanks!

@homu r+ c22e3bf

@homu homu merged commit c22e3bf into rust-num:master Apr 13, 2016
homu added a commit that referenced this pull request Apr 13, 2016
Correct Ratio::is_negative and Ratio::is_positive

Zero is not positive or negative.

This was broken in 8be7e7b.
@homu
Copy link
Contributor

homu commented Apr 13, 2016

⚡ Test exempted - status

remexre pushed a commit to remexre/num that referenced this pull request Jun 1, 2017
doc: follow Rust idiom of trailing comma
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.

3 participants