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 short circuiting behaviour in traverse and traverseFilter #3328

Conversation

gagandeepkalra
Copy link
Contributor

@gagandeepkalra gagandeepkalra commented Feb 28, 2020

Fixed short circuiting behaviour in traverse and traverseFilter for Queue, Chain, Set, Vector and LazyList instances.

resolves #3327

@codecov-io
Copy link

codecov-io commented Feb 28, 2020

Codecov Report

Merging #3328 into master will decrease coverage by 0.09%.
The diff coverage is 80.95%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #3328     +/-   ##
=========================================
- Coverage   93.38%   93.28%   -0.1%     
=========================================
  Files         372      378      +6     
  Lines        7389     7701    +312     
  Branches      219      208     -11     
=========================================
+ Hits         6900     7184    +284     
- Misses        489      517     +28
Flag Coverage Δ
#scala_version_212 93.35% <78.94%> (-0.03%) ⬇️
#scala_version_213 93.06% <80.95%> (?)
Impacted Files Coverage Δ
core/src/main/scala/cats/instances/queue.scala 100% <100%> (ø) ⬆️
...src/main/scala-2.13+/cats/instances/lazyList.scala 100% <100%> (ø)
core/src/main/scala/cats/instances/vector.scala 100% <100%> (ø) ⬆️
core/src/main/scala/cats/data/Chain.scala 98.49% <100%> (+0.01%) ⬆️
...eycats-core/src/main/scala/alleycats/std/set.scala 48.93% <55.55%> (-2.23%) ⬇️
...e/src/main/scala-2.13+/cats/data/ZipLazyList.scala 77.77% <0%> (ø)
.../main/scala-2.13+/cats/data/NonEmptyLazyList.scala 78.18% <0%> (ø)
....13+/cats/kernel/instances/ArraySeqInstances.scala 100% <0%> (ø)
...src/main/scala-2.13+/cats/instances/arraySeq.scala 100% <0%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2573961...2ad7264. Read the comment docs.

@travisbrown
Copy link
Contributor

This looks reasonable to me, but I really wish we had tests to keep us in line in this respect.

Copy link
Contributor

@travisbrown travisbrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just opened #3355 to track the testing issue, and I think we should go ahead and merge this.

@gagandeepkalra
Copy link
Contributor Author

@travisbrown sure, thank you

@travisbrown travisbrown added this to the 2.2.0-M1 milestone Mar 20, 2020
Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks!

@travisbrown travisbrown merged commit 53575e5 into typelevel:master Mar 24, 2020
@travisbrown travisbrown mentioned this pull request Mar 24, 2020
@gagandeepkalra gagandeepkalra deleted the fix/traverseFilter/short-circuiting branch March 25, 2020 01:37
@gagandeepkalra
Copy link
Contributor Author

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TraverseFilter won't short circuit
4 participants