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

Make ListWrapper part of cats-testkit #2645

Closed
ArturGajowy opened this issue Nov 28, 2018 · 3 comments
Closed

Make ListWrapper part of cats-testkit #2645

ArturGajowy opened this issue Nov 28, 2018 · 3 comments

Comments

@ArturGajowy
Copy link
Contributor

All the reasons for ListWrapper's existence and usefulness in cats tests apply to any laws tests users might want to reuse for their own instances. Would you accept a PR moving the class to cats-testkit's src/main?
@kailuowang
Copy link
Contributor

that would be awesome IMO.

ArturGajowy added a commit to ArturGajowy/cats that referenced this issue Nov 29, 2018
@ArturGajowy
Move cats.tests.ListWrapper to cats-testkit
a16c724 
Fixes typelevel#2645
@ArturGajowy ArturGajowy mentioned this issue Nov 29, 2018
Merged
@Alistair-Johnson
Copy link
Contributor

As I mentioned on gitter, I have found an OOM issue with ListWrapper...but so long as I push the PR fix before a release, then all's fine. Just mentioning in case I forget...

But I concur...good idea 👍

@Alistair-Johnson
Copy link
Contributor

So we can't forget... #2648

LukaJCB pushed a commit that referenced this issue Dec 18, 2018
@ArturGajowy
Move cats.tests.ListWrapper to cats-testkit (#2647)
0fcdd0f 
Fixes #2645
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kailuowang @ArturGajowy @Alistair-Johnson