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

Scala 3 #28

Closed
wants to merge 4 commits into from
Closed

Scala 3 #28

wants to merge 4 commits into from

Conversation

mtomko
Copy link
Collaborator

@mtomko mtomko commented Jan 10, 2024

#5

@mtomko mtomko self-assigned this Jan 10, 2024
@mtomko mtomko marked this pull request as ready for review January 10, 2024 23:09
@@ -388,7 +388,7 @@ object PoolQConfig {
// run control
args += (("row-matcher", config.rowMatchFn))
args += (("col-matcher", config.colMatchFn))
if (config.countAmbiguous) {
if config.countAmbiguous then {

Choose a reason for hiding this comment

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

Braces with then?

@nessus42
Copy link

Personally, if I were going to switch to then in if-then-else's, I wouldn't also use curly braces. I'd go completely new-style or stay completely old-style wrt this. I find the mixture to be a bit discombobulating.

OTOH, I have no interest in being a style nazi either.

🐱 🌔 🎱

@mtomko
Copy link
Collaborator Author

mtomko commented Jan 11, 2024

I just run the compiler through with the first set of options described here:

https://docs.scala-lang.org/scala3/guides/migration/tooling-syntax-rewriting.html#the-new-syntax-rewrites

Namely, -new-syntax -rewrite. I manually replaced imports ending with ._ with .*, too.

Because Scala 3's syntactic changes are so new, I decided not to go to the indentation significant route yet. if-then-else plus for-do etc has the advantage (in my opinion) of having fewer parens, which offers some moderate value.

I may well go the route of significant indentation here at some point, though. If people feel like PoolQ should go whole hog, I'm happy to run the next set of compiler syntax rewrites. Perhaps I'll do that anyway and provide it as a commit at the end here, which we could decide to keep or drop.

@mtomko mtomko force-pushed the scala3 branch 2 times, most recently from 6dc9eae to 4eccf03 Compare January 11, 2024 05:12
Copy link
Collaborator Author

@mtomko mtomko left a comment

Choose a reason for hiding this comment

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

I rolled back the indentation-significant syntax changes. There is a bug in scalafmt that I can't work around. See my comment on

scalameta/scalafmt#3653

That issue is closed, I'm going to either open a new one or get them to re-open that one.

Copy link
Member

@tmgreen tmgreen left a comment

Choose a reason for hiding this comment

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

👍

@kitbellew
Copy link

@mtomko
Copy link
Collaborator Author

mtomko commented Feb 20, 2024

Closing in favor of #34

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