Deprecate universal Equiv instance #7186
Conversation
src/library/scala/math/Equiv.scala
Outdated
| trait LowPriorityEquiv { | ||
| self: Equiv.type => | ||
|
|
||
| @deprecated("An implicit universal Equiv instance allows code to compile when it is comparing instances of types for which equality isn't well-defined or implemented (for example, comparing two Function1 instances doesn't generally provide a meaningful result). Use Equiv.universal explicitly instead. If you really want an implicit univeral Equiv intance despite the potential problems, you can write a method `implicit def universalEquiv[T]: Equiv[T] = universal[T]`", "2.12.7") |
There was a problem hiding this comment.
This is a pretty long line. I briefly looked for other examples of long deprecation messages in the code base but didn't see much. Happy to reformat if desired.
There was a problem hiding this comment.
you can look at Ordering.DeprecatedFloatOrdering for an example of a long deprecation message.
There was a problem hiding this comment.
I'd suggesting explaining at any length you like in the Scaladoc, then putting something shorter in the deprecation message that also refers the user to the Scaladoc.
There was a problem hiding this comment.
@SethTisue I attempted to follow your advice. Let me know what you think.
This targets the 2.12 branch. See discussion [here](scala#7164).
ceedubs
force-pushed
the
deprecate-universal-equiv-2.12
branch
from
0aab7e6 to
88e8ae3
Compare
src/library/scala/math/Equiv.scala
Outdated
| * despite the potential problems, you can write a method such as `implicit def universalEquiv[T]: | ||
| * Equiv[T] = universal[T]` | ||
| */ | ||
| @deprecated("Use explicit Equiv.universal instead. See Scaladoc entry for more information.", "2.12.7") |
There was a problem hiding this comment.
Would be nice to put the URL to the entry in the deprecation message.
There was a problem hiding this comment.
@smarter could you please clarify what you mean by "URL to the entry"? Do you mean something like https://www.scala-lang.org/api/2.12.7/scala/math/Equiv$.html#universalEquiv[T]:scala.math.Equiv[T]? I assume that the eventual URL will look like this, but it seems a little fragile.
|
Thanks @ceedubs! I took the liberty of fixing a typo and tightening the wording a bit :-) Hope that's ok! (When merging, let's squash.) |
|
@adriaanm sure, no problem. Although admittedly I'm scratching my head trying to figure out how you had commit rights to my fork. I don't see anyone else having permissions in Settings. Having said that, I've grown wary of this deprecation. See #7187 (comment) This might actually be a case in which it would be easier on users to just remove this without going through a deprecation cycle. |
|
When you submit a PR, there's a checkbox to uncheck if you want to disallow edits. (It's called " Allow edits from maintainers.", and you can also uncheck it after submitting the PR) |
|
Ah, I thought that just applied to the text in the description 😆. It's totally fine I was just confused. |
|
Regarding your concerns with the deprecation, what should we do with this PR? |
|
(I am not @ceedubs but) I think we should deprecate in 2.13, where we can add non-deprecated methods for all of the core types. I don't think it makes sense to deprecate without those available (since there may not be anything to replace the universal |
|
@adriaanm @NthPortal sorry for the lack of activity on this and #7187. I'll try to come back to both of these this weekend. |
|
I'm going to go ahead and close this out until we reach a conclusion on #7187, as I think that the two are pretty intertwined and one probably shouldn't happen without the other. |
6 participants
This targets the 2.12 branch. See discussion here.