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.

Sign up for GitHub

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

Jump to bottom

Consider removing findUpperBound from Arrays #2131

Closed
nventuro opened this issue Mar 16, 2020 · 2 comments
Closed

Consider removing findUpperBound from Arrays #2131

nventuro opened this issue Mar 16, 2020 · 2 comments
Milestone
v3.0

Comments

@nventuro
Copy link
Contributor

As mentioned here, its semantics are very specific to ERC20Snapshot and may not be generally useful.

If we move forward with making this function a private function of ERC20Snapshot, we'd have to figure out how to port the tests for it.
@nventuro nventuro added this to the v3.0 milestone Mar 16, 2020
@nventuro nventuro mentioned this issue Mar 16, 2020
Merged
@abcoathup abcoathup changed the title Consider removing findUperBound from Arrays Consider removing findUpperBound from Arrays Apr 16, 2020
@nventuro
Copy link
Contributor Author

I'm not so sure it's worth removing this from the library: it isn't much more than a binary search, the associated effort with getting rid of it is not small (due to the need to port the tests).

@frangio
Copy link
Contributor

frangio commented Apr 16, 2020

Agreed. Let's keep it.

@frangio frangio closed this as completed Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.0
Development

No branches or pull requests
2 participants
@frangio @nventuro