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

alleycats - Extract[F[_]] the only way to get the syntax is the deprecated one #4642

Open
arturaz opened this issue Aug 5, 2024 · 4 comments · May be fixed by #4670
Open

alleycats - Extract[F[_]] the only way to get the syntax is the deprecated one #4642

arturaz opened this issue Aug 5, 2024 · 4 comments · May be fixed by #4670
Labels
good first issue Issues that are easier to take on for first time contributors

Comments

@arturaz
Copy link

arturaz commented Aug 5, 2024

image

@armanbilge armanbilge added the good first issue Issues that are easier to take on for first time contributors label Aug 5, 2024
@aluscent
Copy link

How can I resolve this issue? I like to contribute; would please provide more details? Should something be added to the documentation?

@satorg
Copy link
Contributor

satorg commented Oct 27, 2024

@aluscent , thank you for offering your help!

If you'd like to, feel free to file a PR. I'd suggest to follow the same code pattern that is used for syntaxes for Empty and Foldable type classes in alleycats.syntax: create ExtractSyntax with corresponding methods, then a new object extact extends ExtractSyntax. Also make sure that alleycats.syntax.all inherits to ExtractSyntax as well.

Personally, I'd probably also consider making trait ExtractSyntax private to the alleycats package, but since neither EmptySyntax nor FoldableSyntax in there are private, I don't have a strong opinion on that.

Also I believe that some tests that could check the new syntax works as expected would be helpful. But it looks like no one've even bothered writing tests for "alleycats", so it is up to you. But you could be the first one, if you'd like to.

aluscent added a commit to aluscent/cats that referenced this issue Oct 29, 2024
aluscent added a commit to aluscent/cats that referenced this issue Oct 29, 2024
@aluscent
Copy link

aluscent commented Oct 29, 2024

Dear @satorg thank you for your guidance.

I fixed the issue and added ExtractSyntax and an object extract - just like empty. I also didn't make it private according to EmptySyntax and FoldableSyntax.

Also I want to write test cases for them, but I will need some guidance again. Should I write tests for the whole alleycats-core? Where should I place them - in alleycats-core/src/test or the tests/shared/src?

@satorg
Copy link
Contributor

satorg commented Nov 23, 2024

@aluscent you don't have to write all the test for sure. You can if you'd like to, but I'd suggest to arrange all unrelated tests in a separate PR.

Regarding the location question – I'm not sure, to be honest. Cats uses the separate tests module for cats-core and cats-kernel, but some other modules use their own test directories for that (e.g. alleycats-laws).

I'd say it is safe to assume that we can start with alleycats-core/src/test for now unless there are objections from other members of the community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are easier to take on for first time contributors
Projects
Development

Successfully merging a pull request may close this issue.

4 participants