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

Fix Inject instance and reduceRightToOption null behavior #1096

Merged
merged 3 commits into from
Jun 8, 2016

Conversation

edmundnoble
Copy link
Contributor

Inject.prj(null) with the provided reflexive Inject instance (Inject[F, F]) returns None, but Inject.inj(null) returns null. I am fairly sure that inj and prj should be mutual inverses, this fixes that. It works the same way now as in Scalaz.

@codecov-io
Copy link

codecov-io commented Jun 7, 2016

Current coverage is 88.88%

Merging #1096 into master will not change coverage

@@             master      #1096   diff @@
==========================================
  Files           226        226          
  Lines          2916       2916          
  Methods        2865       2865          
  Messages          0          0          
  Branches         48         48          
==========================================
  Hits           2592       2592          
  Misses          324        324          
  Partials          0          0          

Sunburst

Powered by Codecov. Last updated by e1a8257...333516a

@edmundnoble edmundnoble changed the title Fix reflexive Inject instance for null Fix Inject instance and reduceRightToOption null behavior Jun 8, 2016
@ceedubs
Copy link
Contributor

ceedubs commented Jun 8, 2016

👍

@non
Copy link
Contributor

non commented Jun 8, 2016

👍 Great catch, thanks!

@non
Copy link
Contributor

non commented Jun 8, 2016

Should we add a law that checks this property, and/or an explicit test of the null behavior with option?

@non
Copy link
Contributor

non commented Jun 8, 2016

I'm going to just merge this -- we can add a test in another PR.

@non non merged commit 948e85c into typelevel:master Jun 8, 2016
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.

4 participants