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

Remove Validated.filter #605

Merged
merged 2 commits into from
Nov 9, 2015
Merged

Remove Validated.filter #605

merged 2 commits into from
Nov 9, 2015

Conversation

ceedubs
Copy link
Contributor

@ceedubs ceedubs commented Nov 7, 2015

I think this was leftover from copy/paste from Xor. It mentions that
it's useful when using Validated in for-comprehensions, but you can't
actually use Validated in for-comprehensions since it doesn't have a
flatMap method :).

Also, Xor no longer has a filter method. It was removed in #276
since using the monoid zero when the predicate doesn't match is rather
arbitrary and can lead to surprises. I think we should follow that
precedent for Validated.

@ceedubs
Remove Validated.filter
71fcdc0 
I think this was leftover from copy/paste from `Xor`. It mentions that
it's useful when using `Validated` in for-comprehensions, but you can't
actually use `Validated` in for-comprehensions since it doesn't have a
`flatMap` method :).

Also, `Xor` no longer has a `filter` method. It was removed in typelevel#276
since using the monoid zero when the predicate doesn't match is rather
arbitrary and can lead to surprises. I think we should follow that
precedent for `Validated`.
@codecov-io
Copy link

Current coverage is 76.29%

Merging #605 into master will not affect coverage as of 6381ac9

@@            master    #605   diff @@
======================================
  Files          160     160       
  Stmts         2194    2194       
  Branches        68      67     -1
  Methods          0       0       
======================================
  Hit           1674    1674       
  Partial          0       0       
  Missed         520     520

Review entire Coverage Diff as of 6381ac9

Powered by Codecov. Updated on successful CI builds.

@non
Copy link
Contributor

non commented Nov 8, 2015

👍

1 similar comment
@adelbertc
Copy link
Contributor

👍

adelbertc added a commit that referenced this pull request Nov 9, 2015
@adelbertc
Merge pull request #605 from ceedubs/rm-validated-filter
912df33 
Remove Validated.filter
@adelbertc adelbertc merged commit 912df33 into typelevel:master Nov 9, 2015
@ceedubs ceedubs deleted the rm-validated-filter branch November 15, 2015 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ceedubs @codecov-io @non @adelbertc @fthomas