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

Define laws for Traversable #373 #449

Open
wants to merge 1 commit into
base: series/1.x
Choose a base branch
from

Conversation

Badmoonz
Copy link

@Badmoonz Badmoonz commented Nov 29, 2020

closes #373

Copy link
Member

@sideeffffect sideeffffect left a comment

Choose a reason for hiding this comment

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

Absolutely awesome job!
I've just one small nit-pick, and if @adamgfraser doesn't have anything to add, I think we can merge 🚀 (but before doing that, we should coordinate somehow with the twin PR #449 )
The jobs are restarted, so 🤞 it will be all green now 🍀


/**
* Composition law
* traverse (Compose . fmap g . f) ta = Compose . fmap (traverse g) . traverse f $ ta
Copy link
Member

Choose a reason for hiding this comment

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

I would be worried that the Haskell names could confuse some users/contributors. I think we should use the ZIO Prelude terminology here, so foreach (instead of traverse) and map (instead of fmap).

@sideeffffect
Copy link
Member

Looking at the builds again, some were not spurious failures. 2.11 may need more explicit type arguments and dotty seems to need more parentheses.
Do you think you could have a look at it @Badmoonz 🙏

@zalbia
Copy link
Contributor

zalbia commented Dec 5, 2020

@Badmoonz I tried adding tests for nested (and product of) Covariant & IdentityBoth` like you said, and ended up with the same 2.11 errors. I don't know how to fix it myself.

zalbia added a commit to zalbia/zio-prelude that referenced this pull request May 11, 2021
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.

Define laws for Traversable
3 participants