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

#nodependencies #29

Merged
merged 8 commits into from
Apr 20, 2015
Merged

#nodependencies #29

merged 8 commits into from
Apr 20, 2015

Conversation

robrix
Copy link
Contributor

@robrix robrix commented Apr 19, 2015

Fixes #28.

@robrix
Copy link
Contributor Author

robrix commented Apr 19, 2015

Ready for review.

@robrix robrix mentioned this pull request Apr 19, 2015
1 task
@rnapier
Copy link
Member

rnapier commented Apr 19, 2015

👍

@robrix
Copy link
Contributor Author

robrix commented Apr 19, 2015

@jspahrsummers Any concerns with this approach?

@robrix
Copy link
Contributor Author

robrix commented Apr 19, 2015

It occurs to me we could make Box private:

  • make a private ResultState enum with analysis and nothing else
  • make Result a public struct with the real API

Then, since Box isn’t exposed by public cases, it has no references in public API at all, and could be private.

@jspahrsummers
Copy link
Member

💯 All of the above sounds awesome. Thanks @robrix!

Hiding the Result cases might make pattern matching slightly more difficult, but Box kind of impaired that anyways (since you can't match through a box, IIRC). And maybe we could implement a custom ~=? Not sure how that works.

@robrix
Copy link
Contributor Author

robrix commented Apr 20, 2015

Correct: you can’t match through anything but an enum or tuple. To the best of my knowledge, ~= is limited to pass/fail, so that wouldn’t allow us to destructure, but we could maybe match against expected error/success values; and analysis recovers destructuring.

Gonna hide Box away now.

@robrix
Copy link
Contributor Author

robrix commented Apr 20, 2015

Alright, it’s done 🀄

@jspahrsummers
Copy link
Member

robrix added a commit that referenced this pull request Apr 20, 2015
@robrix robrix merged commit 1756b77 into master Apr 20, 2015
@robrix robrix deleted the hash-tag-no-dependencies branch April 20, 2015 21:33
@mdiep
Copy link
Contributor

mdiep commented Apr 20, 2015

Can we get a release with this change? 😁

@robrix
Copy link
Contributor Author

robrix commented Apr 20, 2015

@mdiep 👍

@robrix
Copy link
Contributor Author

robrix commented Apr 20, 2015

@mdiep
Copy link
Contributor

mdiep commented Apr 20, 2015

Thank you! 🙏

@robrix
Copy link
Contributor Author

robrix commented Apr 20, 2015

🙇 you’re welcome!

@robrix robrix mentioned this pull request Apr 23, 2015
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.

Proposal: remove Either dependency
4 participants