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

Clean up warnings and scalac options. #491

Merged
merged 3 commits into from
Aug 6, 2019

Conversation

non
Copy link
Contributor

@non non commented Aug 6, 2019

This removes -deprecation warnings for 2.13 (since there are over 100
of them). It also tries to make it a bit easier to see which options
are used for which versions. Finally it removes an unused import.

I did this as an isolated PR because it's small and easy to review.

This removes -deprecation warnings for 2.13 (since there are over 100
of them). It also tries to make it a bit easier to see which options
are used for which versions. Finally it removes an unused import.
@non
Copy link
Contributor Author

non commented Aug 6, 2019

cc @dwijnand @SethTisue

I'm going to tag you all on these PRs that are primarily build-related. Let me know if anything jumps out at you.

non added 2 commits August 6, 2019 00:53
The previous refactor was still a bit hard to read. This should be
easier to understand and is also a bit more compact.
At this point it's probably not worth polishing more.
Copy link
Contributor

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

LGTM. It's an interesting way to organise things, too.

@ashawley
Copy link
Contributor

ashawley commented Aug 6, 2019

I worry this won't work for adding Dotty to the build. After #423 is merged, I predict people will relatively quickly request that Dotty be added as a compiler so there's no backsliding.

@non
Copy link
Contributor Author

non commented Aug 6, 2019

@ashawley I think the strategy will be easily to modify to support dotty. For example:

mk(211 to 214(...),
mk(211 to 213)(...),
mk(212 to 300)(...),
mk(214 to 301)(...))

(I think it's very unlikely we'll ever need more than 99 major versions, but we could always pad the 2/3 by 1000 if we needed to.)

In the worst case (where the ranges need to be disjoint) we can widen that type to IndexedSeq[Int], e.g. (211 to 213) :+ 300 (skipping 214).

@non non merged commit 122cc7a into typelevel:master Aug 6, 2019
@non non deleted the topic/remove-warnings branch August 6, 2019 15:37
@ashawley
Copy link
Contributor

ashawley commented Aug 6, 2019

Dotty versions are of the scheme "0.17.0-RC1", see https://contributors.scala-lang.org/t/announcing-dotty-0-17-0-rc1/3556

@smarter
Copy link
Contributor

smarter commented Aug 7, 2019

Yeah, this will need to be changed for Dotty support. The usual way to do this sort of things is to pattern match on CrossVersion.partialVersion(scalaVersion.value): https://github.com/scala/scala-module-dependency-sample#depend-on-scala-modules-like-a-pro

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants