-
Notifications
You must be signed in to change notification settings - Fork 45
Test everything #48
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
Test everything #48
Conversation
cc6c052
to
ebb8160
Compare
ebb8160
to
f108941
Compare
Also fixes #50 |
@asmeurer This PR makes significant changes to the manip and stat functions of #39, so I've closed that as it doesn't make sense to review such out-dated and somewhat erroneous code. I'm also now ready for review/happy for merging. (IMO we should take the approach that I should just merge my large PRs if you can't review them to a satisfactory standard after a few days + there are no glaring issues. If that was the case I'd be incentivised to make more atomic PRs anywho, so when I inevitably screw something up it'll be easier to identify where I went wrong heh.) |
Yes, it would be OK with me if you do more self merging. I'm not able to review such large PRs in detail in a reasonable amount of time. |
This PR should finishing testing every function in the spec 🎉
Starts from #39. Currently just implements searching tests.Covers everything from the now-closed #39, and finishes out the rest.I'd rather do this all in one go as 1) I'd rather not further chain PRs 2) I'm gunna want to refactor some utils.