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

Scala3 style imports #4677

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

satorg
Copy link
Contributor

@satorg satorg commented Nov 15, 2024

Removes runner.dialectOverride.allowStarWildcardImport = false from .scalafmt.conf then applies scalafmtAll.

UPD.: also re-enables "-Xsource:3" for algebraCore and algebraLaws.

@satorg satorg added the behind-the-scenes appreciated, but not user-facing label Nov 15, 2024
@satorg satorg self-assigned this Nov 15, 2024
@satorg
Copy link
Contributor Author

satorg commented Nov 16, 2024

Looks like it is failing because in #4478 the "-Xsource:3" option was disabled for algebraCore and algebraLaws with the following commit message:

Workaround binary-compatibility issues

🤔

@satorg
Copy link
Contributor Author

satorg commented Nov 16, 2024

Hi @armanbilge,
Do you remember by chance what was the issue with "-Xsource:3" in #4478? I re-enabled it in this PR and all the checks seem passing. Do you think it is safe to leave "-Xsource:3" enabled then?
Thanks!

@armanbilge
Copy link
Member

Huh, that surprises me. I do have a vague recollection of the issue, I believe it related to some methods that were lacking an explicit type, and under -Xsource:3 the type inference works differently, so this would break binary compatibility. And adding an explicit type is impossible, because the binary-compatible explicit type is different for Scala 2 and Scala 3.

If CI is happy, it's probably okay now (?) but it would be really good to understand why ...

@satorg
Copy link
Contributor Author

satorg commented Nov 16, 2024

Yes, it is puzzling quite a bit.. I'm going to convert this Draft to a complete PR and let it stay that way for a while.
Just in order to collect more thoughts and opinions on that and allow some time for pondering all the implications.

@satorg satorg marked this pull request as ready for review November 16, 2024 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behind-the-scenes appreciated, but not user-facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants