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

Par functions for Bitraverse #2750

Merged
merged 2 commits into from
Apr 8, 2019
Merged

Conversation

catostrophe
Copy link
Contributor

@catostrophe catostrophe commented Mar 17, 2019

Added parBitraverse, parBisequence, parLeftTraverse, parLeftSequence

@codecov-io
Copy link

codecov-io commented Mar 17, 2019

Codecov Report

Merging #2750 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2750      +/-   ##
==========================================
+ Coverage   95.34%   95.35%   +0.01%     
==========================================
  Files         363      363              
  Lines        6782     6798      +16     
  Branches      290      286       -4     
==========================================
+ Hits         6466     6482      +16     
  Misses        316      316
Impacted Files Coverage Δ
testkit/src/main/scala/cats/tests/CatsSuite.scala 33.33% <ø> (ø) ⬆️
core/src/main/scala/cats/syntax/parallel.scala 80% <100%> (+9.41%) ⬆️
core/src/main/scala/cats/Parallel.scala 90% <100%> (+1.53%) ⬆️

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 103a698...0c724c8. Read the comment docs.

LukaJCB
LukaJCB previously approved these changes Mar 17, 2019
Copy link
Member

@LukaJCB LukaJCB left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @catostrophe! :)

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.

Thanks a lot for your contribution!
An easy way to clean the commit history up might be hard reset this branch to remote master and then cherry pick your actual change commit 0922a78 then merge the conflicts.
something like
if you don't have an upstream remote yet

git remote add upstream git@github.com:typelevel/cats.git

Then from you branch

git reset --hard upstream/master
git cherry-pick 0922a78

if git reports some conflicts, you can resolve those and run

git add . 
git cherry-pick --continue

then finally

git push -f

Also is it possible that we add more tests? I can see that these tests are similar to the ones ParNonEmptyTraverse has but I am not sure if that's sufficient either. Tests like ParSequence would be nice IMO.

@catostrophe
Copy link
Contributor Author

@kailuowang fixed but the doc build is failing :(

@LukaJCB
Copy link
Member

LukaJCB commented Mar 31, 2019

I restarted the build and cleared the cache, should work now :)

Copy link
Member

@LukaJCB LukaJCB left a comment

Choose a reason for hiding this comment

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

Thanks as always @catostrophe

@LukaJCB LukaJCB requested a review from kailuowang March 31, 2019 16:12
@kailuowang kailuowang merged commit f828924 into typelevel:master Apr 8, 2019
@kailuowang kailuowang added this to the 2.0.0-M1 milestone Apr 18, 2019
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.

4 participants