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

Remove redundant range check #32

Closed

Conversation

sgeisler
Copy link
Contributor

Since u5 is defined in this crate it can be instantiated directly avoiding a redundant range check.

@sgeisler sgeisler requested a review from dongcarl April 23, 2019 14:36
Copy link
Member

@clarkmoody clarkmoody left a comment

Choose a reason for hiding this comment

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

Good catch 👍

@clarkmoody clarkmoody mentioned this pull request Apr 24, 2019
@clarkmoody
Copy link
Member

clarkmoody commented Jul 17, 2019

Can we close this after the work done on #35? Or does this need a simple rebase?

@sgeisler
Copy link
Contributor Author

I thought about that too. It isn't redundant and would just need a rebase, but while it's obvious to us now that the invariants hold it might not be in the future, so maybe leaving it as is is actually best.

I'll close it for now, if you disagree feel free to reopen, I should have put more thought into it before opening the PR (just noticed it when reviewing something else).

@sgeisler sgeisler closed this Jul 17, 2019
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.

None yet

3 participants