Skip to content

Add scala.Try as alias of scala.util.Try to scala package. #7425

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
Closed
wants to merge 2 commits into from
Closed

Add scala.Try as alias of scala.util.Try to scala package. #7425

wants to merge 2 commits into from

Conversation

He-Pin
Copy link
Contributor

@He-Pin He-Pin commented Nov 13, 2018 •
edited
Loading

These classes deserve to be top level. And could help if we want to move some classes in scala.util to dedicated modules.
One concern is serialization stability....

refs:scala/bug#11268

@scala-jenkins scala-jenkins added this to the 2.13.0-RC1 milestone Nov 13, 2018
@He-Pin
Copy link
Contributor Author

He-Pin commented Nov 13, 2018 •
edited
Loading

But maybe the Success and Failure should lives in Try object then?
Or that's OK. At least for me.
I only have XXXResult in my project.

@He-Pin
Copy link
Contributor Author

He-Pin commented Nov 13, 2018

scala/bug#11250

@vitaliihonta
Copy link

@hepin1989 I agree that Try is frequently used so this move will be useful.
But maybe it would be better not to move Try itself but define aliases in scala package object in the same way as for Either?
Something like:

package object scala {
    ...
    type Try[+A] = scala.util.Try[A]
    val Try = scala.util.Try
    type Success[+A] = scala.util.Success[A]
    val Success = scala.util.Success
    type Failure[+A] = scala.util.Failure[A]
    val Failure = scala.util.Failure
    ...
}

Such approach at least won't break source compatibility

@He-Pin
Copy link
Contributor Author

He-Pin commented Nov 14, 2018

@vitaliihonta once moved we could do some extraction of the scala.util , right, maybe?

@He-Pin He-Pin changed the title !lib Move scala.util.Try and scala.util.Either to scala package. Move scala.util.Try and scala.util.Either to scala package. Nov 22, 2018
@dwijnand dwijnand mentioned this pull request Nov 22, 2018
Closed
@lrytz
Copy link
Member

lrytz commented Dec 14, 2018

Had a quick chat about this with @adriaanm. We suggest the following for 2.13 (as it's very late in the game):

  • Add scala.Try type and value aliases (just like for Either), but not for Success/Failure.
  • Move scala.util.Success and scala.util.Failure into the scala.util.Try companion object
  • Add deprecated type/value aliases scala.util.Success = scala.util.Try.Success, same for Failure. The deprecation message should recommend using Try.Success/Try.Failure (instead of importing them).

@He-Pin
Copy link
Contributor Author

He-Pin commented Dec 14, 2018

@lrytz I will address your comment tonight.

@He-Pin
Copy link
Contributor Author

He-Pin commented Dec 14, 2018

@lrytz Should we take the same lockstep, eg move scala.util.Right and scala.util.Left to Either's companion object? As enum is introduced in Scala 3, will this be encoded as enum?

He-Pin
He-Pin commented Dec 14, 2018
View reviewed changes
src/library/scala/package.scala
@@ -143,4 +143,6 @@ package object scala {
type Right[+A, +B] = scala.util.Right[A, B]
val Right = scala.util.Right

type Try[+T] = scala.util.Try[T]
val Try = scala.util.Try
Copy link
Contributor Author

@He-Pin He-Pin Dec 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

as requested.

@He-Pin
Copy link
Contributor Author

He-Pin commented Dec 14, 2018

@Kobenko Could you please help me with the failure? it currently not pass the check you introduced in
#7412, thank you very much.

He-Pin
He-Pin commented Dec 14, 2018
View reviewed changes
test/files/neg/logImplicits.check Outdated
def f = (1 -> 2) + "c"
^
logImplicits.scala:21: applied implicit conversion from (Int, Int) to ?{def +: ?} = final implicit def any2stringadd[A](self: A): any2stringadd[A]
logImplicits.scala:21: applied implicit conversion from (Int, Int) to ?{def +: ?} = implicit def any2stringadd[A](self: A): any2stringadd[A]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, seems it's my problem.

Copy link

Choose a reason for hiding this comment

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

apparently you fixed something and the final flag is no longer needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Kobenko thanks, seems like I switched between branches and it was messed up.

@He-Pin He-Pin changed the title Move scala.util.Try and scala.util.Either to scala package. Add scala.Try as alias of scala.util.Try to scala package. Dec 15, 2018
@He-Pin He-Pin mentioned this pull request Dec 15, 2018
Closed
@He-Pin
Introduce type alias scala.Try for scala.util.Try.
5473bf0 
1. Move `Success` and `Failure` inside the `Try`'s companion object,
2.Add alias for to `scala.util`'s package object and deprecated them in favor of `Scala.Try`, `Try.Success` and `Try.Failure`.
He-Pin
He-Pin commented Dec 15, 2018
View reviewed changes
src/library/scala/util/Try.scala
/**
* Class `Failure[+T]` represent a computation of `T` is failed with a [[Throwable]].
*
* @tparam T Type of the expected result value type.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked it with my local build.

@He-Pin He-Pin mentioned this pull request Dec 17, 2018
Closed
@lrytz
Copy link
Member

lrytz commented Dec 17, 2018

Looks good to me now. Maybe it's better to start a new pull request, so we can start a fresh discussion.

@He-Pin
Copy link
Contributor Author

He-Pin commented Dec 17, 2018

ok

@He-Pin He-Pin closed this Dec 17, 2018
@He-Pin
Copy link
Contributor Author

He-Pin commented Dec 17, 2018

pre: #7549

@SethTisue SethTisue removed this from the 2.13.0-RC1 milestone Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kobenko Kobenko Kobenko left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@He-Pin @vitaliihonta @lrytz @Kobenko @SethTisue @scala-jenkins