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.

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

Add support for Scala 2.13.3 #1857

Merged
merged 1 commit into from Jul 1, 2020
Merged

Add support for Scala 2.13.3 #1857

merged 1 commit into from Jul 1, 2020

Conversation

tgodzik
Copy link
Contributor

@tgodzik tgodzik commented Jun 26, 2020

No description provided.

@tgodzik
Copy link
Contributor Author

tgodzik commented Jun 26, 2020 •
edited by ckipp01

I see 3 issues currently:

  • scalameta regression - already fixed
  • Bloop fails to compile 2.13.3 - this is blocking unfortunately
  • ammonite is not yet released for 2.13.3, but we can just ignore the test for now

@tgodzik tgodzik force-pushed the update-to-scala-2.13.3 branch 2 times, most recently from 4c9941c to 217faef Compare June 26, 2020 17:17
@kpbochenek
Copy link
Contributor

Bloop issue: scalacenter/bloop#1330

@kpbochenek kpbochenek added this to the Metals v0.9.1 milestone Jun 30, 2020
@kpbochenek
Copy link
Contributor

bloop fix is done and will be delivered in 1.4.3 release(hopefully soon 🤞 ).
ammonite with support for 2.13.3: val ammonite = "2.1.4-8-5d0c097" (Ammonite213Suite passes)

@tgodzik tgodzik force-pushed the update-to-scala-2.13.3 branch 2 times, most recently from 49a3852 to 7d54f12 Compare June 30, 2020 21:25
kpbochenek
kpbochenek approved these changes Jun 30, 2020
View reviewed changes
Copy link
Contributor

@kpbochenek kpbochenek left a comment

Choose a reason for hiding this comment

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

LGTM! :)

tgodzik
tgodzik commented Jul 1, 2020
View reviewed changes
build.sbt
def scala2Versions = nonDeprecatedScala2Versions ++ deprecatedScala2Versions

// Scala 3
def nonDeprecatedScala3Versions = Seq(scala3, "0.24.0")
def deprecatedScala3Versions = Seq()
def deprecatedScala3Versions = Seq("0.24.0-RC1")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this back, since I figured it we should not be just dropping it on users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also added 0.23.0, which doesn't influence the tests

ckipp01
ckipp01 approved these changes Jul 1, 2020
View reviewed changes
Copy link
Member

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

mtags/src/main/scala-2/scala/meta/internal/pc/completions/FilenameCompletions.scala
@@ -7,11 +7,8 @@ import scala.collection.immutable.Nil
import scala.util.control.NonFatal

import scala.meta.internal.pc.CompletionFuzzy
import scala.meta.internal.pc.CompletionFuzzy
Copy link
Member

Choose a reason for hiding this comment

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

Random side note, the next release of OrganizeImports will also remove duplicated ones like this 🥳

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 think that was actually removed by that rule 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or maybe it was due to warnings I saw?

Copy link
Member

Choose a reason for hiding this comment

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

O really? Haha, nevermind then. I thought you did it manually.

ckipp01
ckipp01 reviewed Jul 1, 2020
View reviewed changes
build.sbt Outdated
def scala2Versions = nonDeprecatedScala2Versions ++ deprecatedScala2Versions

// Scala 3
def nonDeprecatedScala3Versions = Seq(scala3, "0.24.0")
def deprecatedScala3Versions = Seq()
def deprecatedScala3Versions = Seq("0.23.0", "0.24.0-RC1")
Copy link
Member

@ckipp01 ckipp01 Jul 1, 2020 •
edited

Choose a reason for hiding this comment

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

Wait, actually, with adding 0.23.0 back in here, don't we also need the scala-0.23/ directory with ClassFinder in it in mtags? If not you get the following when trying to cross publish:

[info] Setting Scala version to 0.23.0 on 2 projects.
[info] Excluded 10 projects, run ++ 0.23.0 -v for more details.
[info] Reapplying settings...
[info] set current project to default-86938c (in build file:/Users/ckipp/Documents/scala-workspace/scalameta-org/metals/)
[info] Wrote /Users/ckipp/Documents/scala-workspace/scalameta-org/metals/mtags/target/scala-0.23.0/mtags_0.23.0-0.9.1-SNAPSHOT.pom
[warn] There may be incompatibilities among your library dependencies; run 'evicted' to see detailed eviction warnings.
[info] Compiling 51 Scala sources to /Users/ckipp/Documents/scala-workspace/scalameta-org/metals/mtags/target/scala-0.23.0/classes ...
[error] -- [E008] Member Not Found Error: /Users/ckipp/Documents/scala-workspace/scalameta-org/metals/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerInterfaces.scala:15:34
[error] 15 |import dotty.tools.dotc.reporting.Diagnostic.Error
[error]    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |value Diagnostic is not a member of dotty.tools.dotc.reporting - did you mean reporting.diagnostic?
[error] -- [E006] Unbound Identifier Error: /Users/ckipp/Documents/scala-workspace/scalameta-org/metals/mtags/src/main/scala-3/scala/meta/internal/pc/ScalaPresentationCompiler.scala:523:8
[error] 523 |        ClassFinder.findClassForOffset(params.offset, filename)(
[error]     |        ^^^^^^^^^^^
[error]     |        Not found: ClassFinder
[error] two errors found
[error] (mtags / Compile / compileIncremental) Compilation failed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

och yeah, forgot about :(

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 will remove 0.23 back and add only 0.24.0-RC1 to deprecated. With the next upgrade I will try to keep both versions.

@ckipp01 ckipp01 self-requested a review July 1, 2020 08:24
ckipp01
ckipp01 requested changes Jul 1, 2020
View reviewed changes
Copy link
Member

@ckipp01 ckipp01 left a comment •
edited

Choose a reason for hiding this comment

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

See comment about the scala-0.23 directory. Unless I'm mistaken we hit on this before in #1842

ckipp01
ckipp01 approved these changes Jul 1, 2020
View reviewed changes
Copy link
Member

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

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

Alright, for real this time, LGTM!

@tgodzik tgodzik merged commit dc0cfae into scalameta:master Jul 1, 2020
@tgodzik tgodzik deleted the update-to-scala-2.13.3 branch July 1, 2020 10:28
@alexandru
Copy link

W00t! When are you going to make a release?

@ckipp01
Copy link
Member

ckipp01 commented Jul 2, 2020

W00t! When are you going to make a release?

One was made last night 😉.
https://scalameta.org/metals/blog/2020/07/01/lithium.html

@gabro
Copy link
Member

gabro commented Jul 2, 2020

@alexandru 9h ago 😂 https://twitter.com/scalameta/status/1278450415424671745?s=21

@tgodzik is too fast to follow

@alexandru
Copy link

Awesome, thanks ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kpbochenek kpbochenek kpbochenek approved these changes

@ckipp01 ckipp01 ckipp01 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Metals v0.9.1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tgodzik @kpbochenek @alexandru @ckipp01 @gabro