-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Remove warnings in build and test #24010
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
Conversation
case `2.13` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@odersky It is was put there on purpose so that we can benefit for the latest scala features and fixes but at the same time recognize that it is a Scala 2 file. The warnings will be fixed at some point and we can put -nowarn
instead to avoid them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's very dangerous, because it means that stdlib will also undergo all changes in behavior that are source-coded. Take implicit priority inversion which kicked in at 3.6. Since there was never a proper migration process I think it is prudent to leave it at the old behavior.
What improvements and fixes did you have in mind that would not be available for stdlib if the versions had the proper ordering?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am against @nowarn, that's not a solution. Our warnings should be useful and we should act on them in a timely manner.
I managed to remove all warnings for compiler and test sources by rewriting these sources. Still to do:
|
I now moved the 2.13 source version to its original place but disabled all migration warnings and errors for it. That leaves us with 114 warnings (instead of 2200 before), which starts to be manageable. These warnings should be fixed directly in source. The most common one is a pattern match unchecked warning like this one:
The warning is correct. IterableOnce is covariant but SortedSet is not, so having at the same time an IterableOnce[E] and a SortedSet[E'] does not imply E = E'. So I guess we need to add |
It should come before 3.0
Mostly in comments, the rest was already done.
Still todo: remove warnings in shaded modules
Move 2.13 back to its original place in front of future, but disable all migration warnings and errors for it.
45925f1
to
4248321
Compare
Uh oh!
There was an error while loading. Please reload this page.