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

fix: ensure scala 3 dirs use scala3 dialect for scalafmt #1813

Merged
merged 1 commit into from Jul 19, 2023

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented Jul 6, 2023

I think this is the cause of the error we're seeing in the Scala
Steward

where the scala213 dialect has issues with the Scala 3 code. This small
changes just adds in some fileOverrides to make sure the scala-3 dirs
set the runner.dialect as scala3 instead of scala213.

@ckipp01
Copy link
Member Author

ckipp01 commented Jul 6, 2023

Odd, ./bin/scalafmt --check fails in CI, but locally when running .bin/scalafmt it doesn't format anything... Also does that get dynamically updated in CI or something? I'm not really sure why my change would impact this though 🤔 .

@bjaglin
Copy link
Collaborator

bjaglin commented Jul 7, 2023

Also does that get dynamically updated in CI or something?

🤯 #1784 (comment) (run just before scalafmt) does mutate the faulty project/plugins.sbt. I'll open a PR to run it last (it makes sense anyway as scalafmt is faster to run and more likely to fail).

2 things remain mysterious to me:

  1. Locally, dogfoodScalafixInterfaces' session save does not touch project/plugins.sbt, why does it do it on CI?
  2. Why did the --test picks up the missing LF in this PR (and this PR only)? We might just be witnessing (1) in CI in most cases?

@bjaglin
Copy link
Collaborator

bjaglin commented Jul 7, 2023

Regarding the original trigger for this PR: we are actually back to green since yesterday.

Failing runs (and failing runs only) have "ERROR Steward scalacenter/bloop-config failed", so I assume the problem is related to that. It's comforting that the output triggered by scala-3 files during steward's scalafmt run is not fatal, as scalafmt --test does return successfully in scalafix CI (modulo the weird dogfoodScalafixInterfaces case, which does not happen with steward anyway).

  org.scalasteward.core.io.process$ProcessFailedException: '"SBT_OPTS=-Xmx2048m -Xss8m -XX:MaxMetaspaceSize=512m" mill -i -p /home/runner/scala-steward/workspace/repos/scalacenter/bloop-config/scala-steward.sc show org.scalasteward.mill.plugin.StewardPlugin/extractDeps' exited with code 1.
   Cannot resolve -p. Try `mill resolve _` to see what's available.

I am not sure what changed in bloop-config/steward on July 6th though...

.scalafmt.conf Outdated Show resolved Hide resolved
@ckipp01
Copy link
Member Author

ckipp01 commented Jul 10, 2023

Failing runs (and failing runs only) have "ERROR Steward scalacenter/bloop-config failed", so I assume the problem is related to that. It's comforting that the output triggered by scala-3 files during steward's scalafmt run is not fatal, as scalafmt --test does return successfully in scalafix CI (modulo the weird dogfoodScalafixInterfaces case, which does not happen with steward anyway).

Yup, I noticed this as well. This is because a new release of steward came out with a fix for the actual error we were seeing. There still is an error for this one, but it doesn't fail the run.

I think this is the cause of the error we're seeing in the [Scala
Steward](https://github.com/scalacenter/steward/actions/runs/5470172748/jobs/9959938109#step:5:870)
where the scala213 dialect has issues with the Scala 3 code. This small
changes just adds in some fileOverrides to make sure the scala-3 dirs
set the runner.dialect as scala3 instead of scala213.
Copy link
Collaborator

@bjaglin bjaglin left a comment

Choose a reason for hiding this comment

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

🙇

@ckipp01 ckipp01 merged commit 58b625a into scalacenter:main Jul 19, 2023
8 checks passed
@ckipp01 ckipp01 deleted the scala3Scalafmt branch July 19, 2023 08:14
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.

None yet

2 participants