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

Issue 2304: Missing functions for RWST #3278

Merged
merged 3 commits into from
Jan 30, 2020
Merged

Issue 2304: Missing functions for RWST #3278

merged 3 commits into from
Jan 30, 2020

Conversation

barambani
Copy link
Contributor

addresses the last bit of #2304

@codecov-io
Copy link

codecov-io commented Jan 30, 2020 •
edited
Loading

Codecov Report

Merging #3278 into master will increase coverage by 0.19%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3278      +/-   ##
==========================================
+ Coverage   92.95%   93.14%   +0.19%     
==========================================
  Files         377      378       +1     
  Lines        7550     7576      +26     
  Branches      193      203      +10     
==========================================
+ Hits         7018     7057      +39     
+ Misses        532      519      -13
Flag Coverage Δ
#scala_version_212 93.39% <33.33%> (?)
#scala_version_213 92.91% <33.33%> (-0.04%) ⬇️
Impacted Files Coverage Δ
...in/scala/cats/data/IndexedReaderWriterStateT.scala 95.19% <33.33%> (-1.84%) ⬇️
core/src/main/scala/cats/Show.scala 100% <0%> (ø) ⬆️
core/src/main/scala/cats/instances/tuple.scala 100% <0%> (ø) ⬆️
core/src/main/scala/cats/data/Const.scala 100% <0%> (ø) ⬆️
core/src/main/scala/cats/data/Validated.scala 100% <0%> (ø) ⬆️
...cala-2.12/cats/kernel/compat/TraversableOnce.scala 0% <0%> (ø)
core/src/main/scala/cats/data/NonEmptyVector.scala 99.17% <0%> (+0.01%) ⬆️
core/src/main/scala/cats/data/NonEmptyList.scala 98.72% <0%> (+0.01%) ⬆️
core/src/main/scala/cats/data/NonEmptySet.scala 97.64% <0%> (+0.02%) ⬆️
core/src/main/scala/cats/instances/option.scala 98.3% <0%> (+0.02%) ⬆️
... and 9 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 9cfd973...5d7e6bb. Read the comment docs.

LukaJCB
LukaJCB previously approved these changes Jan 30, 2020
View reviewed changes
@LukaJCB
Copy link
Member

LukaJCB commented Jan 30, 2020

Thanks @barambani!

travisbrown
travisbrown previously approved these changes Jan 30, 2020
View reviewed changes
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.

One minor comment about the unrelated change, but otherwise looks good to me!

tests/src/test/scala/cats/tests/FoldableSuite.scala Outdated
@@ -151,7 +151,7 @@ abstract class FoldableSuite[F[_]: Foldable](name: String)(implicit ArbFInt: Arb
fa.collectFold(pf) should ===(fa.toList.collect(pf).fold(m.empty)(m.combine))

def g(a: String): Option[String] = Some(a).filter(f)
fa.collectSomeFold(g) should ===(fa.toList.filter(f).fold(m.empty)(m.combine))
fa.collectFoldSome(g) should ===(fa.toList.filter(f).fold(m.empty)(m.combine))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd intentionally left this testing the deprecated version for the sake of coverage and safety (I got bitten pretty badly by an untested deprecated method in Circe 0.12.0). I think we're probably okay changing this instance now, but we should have a policy for what to do in these cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know it, happy to revert. It's not relevant for the issue. I will comment there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! (I should have done that already).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the review.

@travisbrown travisbrown added this to the 2.2.0-M1 milestone Jan 30, 2020
@barambani barambani dismissed stale reviews from travisbrown and LukaJCB via f9bac65 January 30, 2020 14:07
@LukaJCB LukaJCB merged commit bb7a180 into typelevel:master Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LukaJCB LukaJCB LukaJCB approved these changes

@travisbrown travisbrown travisbrown approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
2.2.0-M1
Development

Successfully merging this pull request may close these issues.
4 participants
@barambani @codecov-io @LukaJCB @travisbrown