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

#2947 Added bifold to Bifoldable typeclass #3088

Merged
merged 4 commits into from
Nov 5, 2019

Conversation

Twizty
Copy link
Contributor

@Twizty Twizty commented Sep 27, 2019

Implemented #2947

@Twizty
Copy link
Contributor Author

Twizty commented Sep 27, 2019

java.lang.RuntimeException: Could not run jekyll, error: 1 is it a known issue or did I do something wrong?

@Twizty Twizty closed this Sep 27, 2019
@LukaJCB
Copy link
Member

LukaJCB commented Oct 16, 2019

Hey @Twizty, what's the reason you closed this? I think it was a fine PR. Sorry no one responded to you, not sure what went wrong there, I restarted the Travis job to see if it was just a temporary error :)

@LukaJCB LukaJCB reopened this Oct 16, 2019
@codecov-io
Copy link

codecov-io commented Oct 16, 2019

Codecov Report

Merging #3088 into master will decrease coverage by 0.33%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3088      +/-   ##
==========================================
- Coverage   93.57%   93.24%   -0.34%     
==========================================
  Files         371      375       +4     
  Lines        7097     7293     +196     
  Branches      185      196      +11     
==========================================
+ Hits         6641     6800     +159     
- Misses        456      493      +37
Flag Coverage Δ
#scala_version_212 93.57% <100%> (ø) ⬆️
#scala_version_213 90.89% <100%> (?)
Impacted Files Coverage Δ
core/src/main/scala/cats/Bifoldable.scala 91.66% <100%> (+0.75%) ⬆️
...e/src/main/scala-2.13+/cats/data/ZipLazyList.scala 77.77% <0%> (ø)
...src/main/scala-2.13+/cats/instances/lazyList.scala 100% <0%> (ø)
....13+/cats/kernel/instances/LazyListInstances.scala 100% <0%> (ø)
.../main/scala-2.13+/cats/data/NonEmptyLazyList.scala 62.5% <0%> (ø)
...in/scala/cats/data/AbstractNonEmptyInstances.scala 100% <0%> (+3.44%) ⬆️

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 6d191a5...b173026. Read the comment docs.

LukaJCB
LukaJCB previously approved these changes Oct 16, 2019
kailuowang
kailuowang previously approved these changes Oct 18, 2019
Copy link
Contributor

@kailuowang kailuowang left a comment

Choose a reason for hiding this comment

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

i fixed your comment @LukaJCB , now pending build green

@LukaJCB
Copy link
Member

LukaJCB commented Oct 18, 2019

Needs a scalafmt run

@travisbrown
Copy link
Contributor

@kailuowang @LukaJCB I think that last merge got this out of sync, which makes review difficult, since there are a lot of extraneous changes in the diff—mind if I rebase to clean it up?

@LukaJCB
Copy link
Member

LukaJCB commented Nov 5, 2019

Go for it @travisbrown

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 think ideally I'd like to see a test here for something like Tuple2 but I'd be happy to see it merged as-is.

@LukaJCB LukaJCB merged commit 5ad3c40 into typelevel:master Nov 5, 2019
@travisbrown travisbrown added this to the 2.1.0-RC1 milestone Nov 5, 2019
gagandeepkalra added a commit to gagandeepkalra/cats that referenced this pull request Jan 9, 2020
rossabaker pushed a commit that referenced this pull request Feb 4, 2020
* backported #3088 (adding `bifold` to `Bifoldable` typeclass) to scala_2.11

* addressed review feedback regarding name change
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.

5 participants