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

FreeApplicative#hoist is same as compile #524

Merged
merged 3 commits into from
Sep 13, 2015
Merged

FreeApplicative#hoist is same as compile #524

merged 3 commits into from
Sep 13, 2015

Conversation

adelbertc
Copy link
Contributor

No description provided.

@codecov-io
Copy link

Current coverage is 64.57%

Merging #524 into master will decrease coverage by -0.04% as of a91e5ed

@@            master    #524   diff @@
======================================
  Files          157     157       
  Stmts         2416    2419     +3
  Branches        66      66       
  Methods          0       0       
======================================
+ Hit           1561    1562     +1
  Partial          0       0       
- Missed         855     857     +2

Review entire Coverage Diff as of a91e5ed

Powered by Codecov. Updated on successful CI builds.

@fthomas
Copy link
Member

fthomas commented Sep 11, 2015

compile requires an Applicative[G] but hoist does not. If they are the same why don't we remove the one which is more restricted?

@adelbertc
Copy link
Contributor Author

Good catch - Applicative constraint was not necessary :-) The 2 are the same

ceedubs
ceedubs reviewed Sep 12, 2015
View reviewed changes
free/src/main/scala/cats/free/FreeApplicative.scala
@@ -45,7 +38,8 @@ sealed abstract class FreeApplicative[F[_], A] extends Product with Serializable
final def fold(implicit F: Applicative[F]): F[A] =
foldMap(NaturalTransformation.id[F])

final def compile[G[_]](f: F ~> G)(implicit G: Applicative[G]): FA[G, A] =
/** Interpreter this FreeApplicative algebra into another using FreeApplicative. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor typo here. I think you probably meant "Interpret this..."

@ceedubs
Copy link
Contributor

ceedubs commented Sep 13, 2015

👍

1 similar comment
@non
Copy link
Contributor

non commented Sep 13, 2015

👍

non added a commit that referenced this pull request Sep 13, 2015
@non
Merge pull request #524 from adelbertc/freeapplicative-remove-hoist
7be80e5 
FreeApplicative#hoist is same as compile
@non non merged commit 7be80e5 into typelevel:master Sep 13, 2015
@non non removed the in progress label Sep 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@adelbertc @codecov-io @fthomas @ceedubs @non