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

Add Semigroup instance for OneAnd #545

Merged
merged 2 commits into from
Sep 27, 2015
Merged

Conversation

liff
Copy link
Contributor

@liff liff commented Sep 27, 2015

Add Semigroup instance for OneAnd based on its SemigroupK.

I couldn't find any examples on how to write tests for this, so any help is appreciated.

@codecov-io
Copy link

Current coverage is 75.48%

Merging #545 into master will decrease coverage by -0.04% as of b82fa08

@@            master    #545   diff @@
======================================
  Files          153     153       
  Stmts         2063    2064     +1
  Branches        68      68       
  Methods          0       0       
======================================
  Hit           1558    1558       
  Partial          0       0       
- Missed         505     506     +1

Review entire Coverage Diff as of b82fa08

Powered by Codecov. Updated on successful CI builds.

@ceedubs
Copy link
Contributor

ceedubs commented Sep 27, 2015

👍 (assuming Travis turns green)

There's a gitter conversation going on as to whether we might support an automatic SemigroupK => Semigroup conversion with some help from export-hook. The reason export-hook would probably need to be involved is that such a conversion currently needs to live in the Semigroup companion object, which lives in Algebra while SemigroupK lives in Cats. I suppose another option is to move SemigroupK into Algebra. I'm curious what @non thinks about these possibilities. But I think we should move forward with this PR for now and talk about the other possibilities as long-term possibilities.

@non
Copy link
Contributor

non commented Sep 27, 2015

I think supporting export-hook in algebra is a good idea, but I think we should move forward in Cats until that is all set up. 👍

@non
Copy link
Contributor

non commented Sep 27, 2015

@liff I think the checkAll calls you made are fine. They will test that the semigroups work.

non added a commit that referenced this pull request Sep 27, 2015
Add Semigroup instance for OneAnd
@non non merged commit 682a394 into typelevel:master Sep 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants