-
Notifications
You must be signed in to change notification settings - Fork 278
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
Implement changes from Intellij Code Inspection #3104
Conversation
96eba75
to
aa761c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I just added some comments. For some of the unused stuff we could add compiler options and verify that on CI with scalafix. We do that in Metals via:
val src = Source.fromFile(atFile) | ||
try builder ++= src.getLines() | ||
finally src.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also use scala.util.Using
val src = Source.fromFile(atFile) | |
try builder ++= src.getLines() | |
finally src.close() | |
builder ++= Using.resource(Source.fromFile(atFile))(_.getLines()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, thank you, didn't know about this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's only available for scala 2.13. added this package for 2.12:
https://index.scala-lang.org/bigwheel/util-backports/util-backports/2.1
val src = scala.io.Source.fromURL(url) | ||
try src.getLines().mkString("", "\n", "\n") | ||
finally src.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val src = scala.io.Source.fromURL(url) | |
try src.getLines().mkString("", "\n", "\n") | |
finally src.close() | |
builder ++= Using.resource(scala.io.Source.fromURL(url))(_.getLines().mkString("", "\n", "\n")) |
or something along these lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... For some of the unused stuff we could add compiler options ...
thank you, added the compiler options. let's do scalafix later, if you don't mind; not very experienced with it.
val src = Source.fromFile(atFile) | ||
try builder ++= src.getLines() | ||
finally src.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, thank you, didn't know about this
val src = scala.io.Source.fromURL(url) | ||
try src.getLines().mkString("", "\n", "\n") | ||
finally src.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done as well
No description provided.