Support error comparison

cdwort commented 6 years ago

This is currently a breaking change!!

Fixes #50.

This PR changes the interface of equivalent_to? to support only two procs (compare and error_compare). While this is the interface that I would write now, I imagine you'll want to keep backwards compatibility.

What's your preferred means of doing that? Change the equivalent_to? method to be:

def equivalent_to(other, error_comparator=nil, &comparator)

If that looks good to y'all, I'll update this PR to use ^^ interface.

Thanks!!

coveralls commented 6 years ago

Coverage decreased (-0.5%) to 98.598% when pulling 62c3fb99c52c97f723a150ee260eabb29b44e027 on cdwort:support_error_comparison into 25be7a23ffb583adaf9a0bf9b03754b114f0be11 on github:master.

cdwort commented 6 years ago

Lol, :'( I see the coveralls comment... and I can't seem to find the way to see where the coverage failed... although seems likely that the setup method error_compare is what I'm lacking.

coveralls commented 6 years ago

Coverage increased (+0.009%) to 99.065% when pulling 1f9a48782a71bc8f92eaecb2690da1c8075c53a1 on cdwort:support_error_comparison into 25be7a23ffb583adaf9a0bf9b03754b114f0be11 on github:master.

coveralls commented 6 years ago

Coverage increased (+0.009%) to 99.065% when pulling 1f9a48782a71bc8f92eaecb2690da1c8075c53a1 on cdwort:support_error_comparison into 25be7a23ffb583adaf9a0bf9b03754b114f0be11 on github:master.

joeltaylor commented 5 years ago

@cdwort are you still planning on wrapping this one up? If not, I'm happy to jump in to help out.

izuzak commented 3 years ago

:wave: @zerowidth @cdwort Hello from the future! 😸2️⃣0️⃣2️⃣1️⃣

I agree this would be a neat feature to have in Scientist, so I updated this PR based on the review comments from 2018. @cdwort I hope you do mind me pushing directly to your branch to make these updates 😅

2a80f9c changes the naming from error_compare to compare_errors
1bcbab1 fixes the failing tests
c42ca0e updates the README with instructions on the using compare_errors

Could I trouble you to re-review this PR and see if there's anything else we want to do? 🙏

Two notes:

*Travis doesn't seem to be doing its thing anymore -- no checks were created on the commits I pushed, and I see no checks were created on the PR that was most recently merged in this repository. I'm guessing something broke in the meantime, but I don't have admin access to this repo so that I can investigate. The tests are running fine locally ✅, but if we want to fix Travis builds for the website, someone with admin access would need to do that. I'd also be happy to take a look if you'd be brave enough to give me admin access 😬
As @cdwort mentioned, this is breaking change due to the change in the interface. With that in mind, the new version we release after merging this would need to bump the major version number. Personally, I like the new interface and I don't think releasing a new major version would be a problem. But I'd also be okay with considering other approaches if you don't feel like making a breaking change here. :v:

Thanks! :bow:

github / scientist

Support error comparison #77