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

Clean up constructors Arrow Core Datatypes. #344

Merged
merged 6 commits into from Feb 16, 2021
Merged

Conversation

nomisRev
Copy link
Member

This PR deprecates up all duplicated constructors, and deprecates all just constructors.

This leaves behind only the actual constructors, and the extension constructors.

All constructors of Either after removal of deprecated methods:

val x: Either<String, Int> = Either.Right(1)
val y: Either<String, Int> = Either.Left("one")
val x2: Either<String, Int> = 1.right()
val y2: Either<String, Int> = "one".left()

Copy link
Contributor

@franciscodr franciscodr left a comment

Choose a reason for hiding this comment

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

Thanks @nomisRev!

@@ -183,7 +183,7 @@ sealed class Eval<out A> : EvalOf<A> {
fun raise(t: Throwable): Eval<Nothing> =
defer { throw t }

val unit: Eval<Unit> = Now(kotlin.Unit)
val Unit: Eval<Unit> = Now(kotlin.Unit)
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 was uppercased on 0.11.0

Copy link
Member

Choose a reason for hiding this comment

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

Do we want those to be upper-cased or lowercased in Arrow for other libs and API's?

Copy link
Member Author

@nomisRev nomisRev Feb 15, 2021

Choose a reason for hiding this comment

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

Good point @i-walker! After comparing with the other (smart) constructors in Arrow, I think we should deprecate all of these in favor of their right hand side implementations.

val Unit: Eval<Unit> = Now(kotlin.Unit)
val True: Eval<Boolean> = Now(true)
val False: Eval<Boolean> = Now(false)
val Zero: Eval<Int> = Now(0)
val One: Eval<Int> = Now(1)

If all agree I can add this change in this PR as well.

Copy link
Member

Choose a reason for hiding this comment

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

I am sorry, I saw the reply after merging it.
Let's move that to a different PR 😄

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 @nomisRev 🙌 , left a general question below on naming conventions.

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