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

Tweak deprecation messages for Ordering[{Float,Double}] #8191

Merged

Conversation

@NthPortal
Copy link
Contributor

commented Jun 30, 2019

Followup to this comment. Sorry it took so long @dwijnand.

@NthPortal NthPortal requested review from dwijnand and Ichoran Jun 30, 2019

@scala-jenkins scala-jenkins added this to the 2.13.1 milestone Jun 30, 2019

@lrytz
Copy link
Member

left a comment

Thank you :)

@lrytz lrytz merged commit b2a4255 into scala:2.13.x Jul 1, 2019

3 checks passed

cla @NthPortal signed the Scala CLA. Thanks!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
validate-main [9008] SUCCESS. Took 35 min.
Details
@dwijnand

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

No worries, thanks @NthPortal :)

@NthPortal NthPortal deleted the NthPortal:topic/ordering-double-deprecation-message branch Jul 1, 2019

@som-snytt

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2019

Just wanted to say, I think it's great we're using gender-neutral singular-their.

The other way to order doubles is just to say, Make mine a double, and my friend'll have the same.

I forgot to ask if I followed the instructions correctly:

https://github.com/scala/scala/pull/8219/files#diff-91e1dcaa2b97f51d359c32d0ec8c36e0R30

It did actually take me a couple of tries to find the aforementioned antecedent. I went to Ordering, scrolled down, hit the bottom, tried the deprecated thing, then TotalOrdering, then known subclasses.

In Ordering, there are a bunch of traits for FooOrdering, so you skim that, nothing says Double, then the value members are not in alphabetical order, they start with Option, I didn't see Double snuck in further down in the middle.

If you're unhappy enough to write "double" in the search, Double is a member of Ordering way down in the right hand side, but not actually discoverable.

Revisiting the message text, and adding line breaks:

scala> implicitly[Ordering[Double]]
                 ^
       warning: object DeprecatedDoubleOrdering in object Ordering is deprecated
(since 2.13.0): There are multiple ways to order Doubles (Ordering.Double.TotalOrdering,
Ordering.Double.IeeeOrdering). Specify one by using a local import, assigning an implicit 
val, or passing it explicitly. See the documentation for details.
res0: Ordering[Double] = scala.math.Ordering$DeprecatedDoubleOrdering$@793d163b

scala> implicitly[Ordering[Double]]
                 ^
       warning: object DeprecatedDoubleOrdering in object Ordering is deprecated 
(since 2.13.0): Use either Ordering.Double.TotalOrdering or Ordering.Double.IeeeOrdering. 
See the documentation for Ordering.Double. Example usage: import Ordering.Double.TotalOrdering

I don't know if that's better, but I'm running a test at the office that takes a while to finish. My suggestion is to put the overview explanation on Double, then repeat the differences on each flavor: Next time I run my test, I'll do a quickie PR in case it does navigate more easily.

@NthPortal

This comment has been minimized.

Copy link
Contributor Author

commented Jul 16, 2019

I also really like singular "their", but in this case it was referring to TotalOrdering and IeeeOrdering, which is plural.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.