Skip to content

[nomerge] remove unused import.#8212

Merged
SethTisue merged 1 commit intoscala:2.12.xfrom
takayahilton:remove-unused-import
Jul 15, 2019
Merged

[nomerge] remove unused import.#8212
SethTisue merged 1 commit intoscala:2.12.xfrom
takayahilton:remove-unused-import

Conversation

@takayahilton
Copy link
Contributor

@takayahilton takayahilton commented Jul 10, 2019

No description provided.

@scala-jenkins scala-jenkins added this to the 2.12.9 milestone Jul 10, 2019
hrhino
hrhino approved these changes Jul 10, 2019
View reviewed changes
Copy link
Contributor

@hrhino hrhino left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@SethTisue
Copy link
Member

SethTisue commented Jul 10, 2019 •
edited
Loading

This is outstanding, thank you!

I think it might be hard to merge it onto the 2.13.x branch. Can I interest you in marking this one [nomerge] and also submitting a separate 2.13 PR?

Is there a possibility of guarding against regressions in this area by enabling -Ywarn-unused:imports in our build?

@takayahilton
Copy link
Contributor Author

takayahilton commented Jul 10, 2019

@SethTisue
ok.
I will submit a 2.13 PR.
And also add scalac option to -Ywarn-unused: imports.

@takayahilton takayahilton force-pushed the remove-unused-import branch from 459441b to 011d379 Compare July 11, 2019 03:50
@takayahilton takayahilton changed the title remove unused import. [nomerge] remove unused import. Jul 11, 2019
@takayahilton takayahilton force-pushed the remove-unused-import branch from 011d379 to 15caff0 Compare July 11, 2019 06:10
@som-snytt
Copy link
Contributor

som-snytt commented Jul 11, 2019 •
edited
Loading

On 2.13, #6964 but I think it isn't permanent yet. I see a comment about waiting for re-starr.

The similar PR was #6806

Note that it's -Xlint on the 2.13 PR.

@dwijnand

This comment has been minimized.

Sign in to view
@dwijnand dwijnand force-pushed the remove-unused-import branch from 15caff0 to cdfc331 Compare July 12, 2019 07:11
dwijnand
dwijnand approved these changes Jul 12, 2019
View reviewed changes
build.sbt
fork in IntegrationTest := true,
// enable this in 2.13, when tests pass
//scalacOptions in Compile += "-Yvalidate-pos:parser,typer",
scalacOptions -= "-Ywarn-unused:imports",
Copy link
Member

@dwijnand dwijnand Jul 12, 2019

Choose a reason for hiding this comment

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

I'm fine with doing this, but how come you did it?

Copy link
Contributor Author

@takayahilton takayahilton Jul 12, 2019

Choose a reason for hiding this comment

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

I don't know how reason, but partest fails when adding -Ywarn-unused: imports.

Copy link
Contributor

@som-snytt som-snytt Jul 12, 2019

Choose a reason for hiding this comment

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

The 2.13 PR does something similar. Test compilations receive these args, too, with fatal warnings.

@som-snytt
Copy link
Contributor

som-snytt commented Jul 12, 2019

BTW, I'm preparing a quick update to 2.13 since the last clean-up PR.

@diesalbla diesalbla added the internal not resulting in user-visible changes (build changes, tests, internal cleanups) label Jul 15, 2019
@SethTisue SethTisue merged commit f9f64a7 into scala:2.12.x Jul 15, 2019
@takayahilton takayahilton deleted the remove-unused-import branch July 16, 2019 07:56
@som-snytt
Copy link
Contributor

som-snytt commented Sep 5, 2019

Thanks again for setting the flag. It was so nice seeing the extra help in my sbt window today. I was "optimizing imports" as they say in IDE lingo.

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

Reviewers

@som-snytt som-snytt som-snytt left review comments

@dwijnand dwijnand dwijnand approved these changes

+2 more reviewers

@diesalbla diesalbla diesalbla approved these changes

@hrhino hrhino hrhino approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@SethTisue SethTisue

Labels

internal not resulting in user-visible changes (build changes, tests, internal cleanups)

Projects

None yet

Milestone

2.12.9

Development

Successfully merging this pull request may close these issues.

7 participants

@takayahilton @SethTisue @som-snytt @dwijnand @diesalbla @hrhino @scala-jenkins