Skip to content
This repository has been archived by the owner on Feb 24, 2021. It is now read-only.

Deprecate @higherkind & @extension for Either #272

Merged
merged 32 commits into from Dec 20, 2020

Conversation

nomisRev
Copy link
Member

@nomisRev nomisRev commented Dec 16, 2020

  • Eq (EqK, EqK2)
  • Order
  • Hash
  • Show
  • Semigroup (SemigroupK)
  • Monoid
  • Functor
  • Apply
  • Applicative
  • ApplicativeError
  • Monad
  • MonadError
  • Traverse
  • BiTraverse
  • Foldable
  • BiFoldable
  • BiFunctor
  • BiCrosswalk

Needs to be merged together with:

@nomisRev nomisRev marked this pull request as ready for review December 18, 2020 18:36
@nomisRev nomisRev requested a review from a team December 18, 2020 18:36
Comment on lines +1165 to +1169
@Deprecated("Use the inline version. Hidden for binary compat", level = DeprecationLevel.HIDDEN)
suspend inline fun <R> catch(f: suspend () -> R): Either<Throwable, R> =
catch { f() }

inline fun <R> catch(f: () -> R): Either<Throwable, R> =
Copy link
Member Author

Choose a reason for hiding this comment

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

This breaks the API in some places where you pass a suspend lambda directly to the catch function, I think the Kotlin compiler has the same limitation for suspend method references..

See arrow-kt/arrow-fx#354 as example of this breakage.

Copy link
Member

@1Jajen1 1Jajen1 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks! 👏

So our approach for now is copy the (deprecated) kapt output, remove kapt, add meta?

Copy link
Member

@i-walker i-walker left a comment

Choose a reason for hiding this comment

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

🙌 Thanks Simon

@1Jajen1
Copy link
Member

1Jajen1 commented Dec 19, 2020

Looking at how arrow-kt/arrow-fx#354 fails this seems to cause some other problems as well :/

@nomisRev
Copy link
Member Author

So our approach for now is copy the (deprecated) kapt output, remove kapt, add meta?

Yes exactly. That's the most graceful way to get rid of the things we want to get rid of for 1.0. Almost all signatures can be preserved, except traverse which relies on Applicative. So that results in a couple of signatures instead, which need to be differentiated by function name atm due to a bug in the compiler where it cannot differentiate on the return type of the lambda.

@1Jajen1
Copy link
Member

1Jajen1 commented Dec 19, 2020

due to a bug in the compiler where it cannot differentiate on the return type of the lambda.

I think this is fixable in some cases with a custom @jvmName iirc.

@nomisRev
Copy link
Member Author

nomisRev commented Dec 19, 2020

I originally thought so too, but we asked in #compiler on Kotlin Slack and doesn't seem to be the case. See https://youtrack.jetbrains.com/issue/KT-43872

The definition is valid, but the compiler doesn't know which one to pick from the user site :(

@nomisRev nomisRev marked this pull request as draft December 19, 2020 19:32
@nomisRev nomisRev marked this pull request as ready for review December 19, 2020 19:33
@nomisRev nomisRev merged commit 4e573bd into master Dec 20, 2020
@nomisRev nomisRev deleted the sv-deprecate-extensions-either branch December 20, 2020 22:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants