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

Make kittens officially a Cats module (still separate repo) #2361

Merged
merged 1 commit into from
Aug 3, 2018

Conversation

kailuowang
Copy link
Contributor

@kailuowang kailuowang commented Aug 3, 2018

I had a discussion with @milessabin see (typelevel/kittens#96). We think that making kittens an official cats module (still in its own separate repo) would be a good idea. @milessabin moved it to under github typelevel. I'm proposing this to the Cats community through this PR.

Update: I updated the title to further clarify the intention.

I had a discussion with @milessabin see (typelevel/kittens#96). We think that making kittens an official cats module (still in its own separate repo) would be a good idea. @milessabin moved it to under github typelevel.  I'm proposing this to the Cats community through this PR.
@ceedubs
Copy link
Contributor

ceedubs commented Aug 3, 2018

I'm a bit hesitant on this. Kittens depends on Shapeless, right? Here are some concerns that I would have:

  • The Cats builds are already cross-building against several Scala versions and Scala.js. I think that them potentially needing to cross-compile against multiple Shapeless versions someday would be rough (in a combinatorial way).
  • Because of its more complex usage of the type system and heavier usage of macros, I could see Shapeless being quicker to drop older versions of Scala. This could get tricky to try to not cross-build just the kittens module for some scala versions.

I'm happy to see kittens in the typelevel org, but I question the cost/benefit ratio of making it a module within the cats repo.

@kailuowang
Copy link
Contributor Author

@ceedubs not a module inside cats repo, it's still in its own repo with its own release cycle just like the other 3 "outside" modules cats-mtl, cats-effect and mouse.

@kailuowang kailuowang changed the title Make kittens officially a Cats module Make kittens officially a Cats module (still separate repo) Aug 3, 2018
@tpolecat
Copy link
Member

tpolecat commented Aug 3, 2018

Does "official cats module" mean anything other than appearing on this bullet list? This is fine, it's just probably worth clarifying.

@kailuowang
Copy link
Contributor Author

@tpolecat that's mostly it. Although I'd say that cats modules get a commitment from Cats maintainers to help to maintain them.

@ceedubs
Copy link
Contributor

ceedubs commented Aug 3, 2018

@kailuowang ah okay. I understand. I'm all for this then.

I do have a concern about kittens derivation that has prevented me from using it and I've been meaning to look for a discussion of, but I'll pursue that separately.

@LukaJCB
Copy link
Member

LukaJCB commented Aug 3, 2018

Regarding commitment to maintain, I'd happily help out, but I fear my shapeless skills really aren't up to par. I do think kittens is an important project that we (heh) should give more focus, so I really like this move :)

@codecov-io
Copy link

codecov-io commented Aug 3, 2018

Codecov Report

Merging #2361 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2361   +/-   ##
=======================================
  Coverage   95.01%   95.01%           
=======================================
  Files         349      349           
  Lines        5993     5993           
  Branches      223      225    +2     
=======================================
  Hits         5694     5694           
  Misses        299      299

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 38c52ad...1c632d8. Read the comment docs.

@kailuowang
Copy link
Contributor Author

seems that we have enough approval. It's official!

@kailuowang kailuowang merged commit a23f80a into master Aug 3, 2018
@kailuowang kailuowang added this to the 1.3 milestone Aug 16, 2018
@kailuowang kailuowang deleted the kailuowang-patch-1 branch October 19, 2018 13:34
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.

7 participants