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

Resolve: improve detection of unused imports and other miscellaneous improvements #31321

Merged
merged 9 commits into from
Feb 5, 2016

Conversation

jseyfried
Copy link
Contributor

The first commit improves detection of unused imports -- it should have been part of #30325. Right now, the unused import in the changed test would not be reported.

The rest of the commits are miscellaneous, independent clean-ups in resolve that I didn't think warranted individual PRs.

r? @nrc

@jseyfried jseyfried force-pushed the cleanup branch 4 times, most recently from 854b80e to ca98356 Compare January 31, 2016 13:40
@jseyfried
Copy link
Contributor Author

cc @petrochenkov

@nrc
Copy link
Member

nrc commented Feb 1, 2016

Is the first commit a breaking change? Could you add [breaking-change] to the commit if it is?

@jseyfried
Copy link
Contributor Author

I'm not sure if it counts as a breaking change -- it would only cause breakage with #[deny(unused_imports)]. #30325 wasn't marked as breaking (perhaps incorrectly).

@nrc
Copy link
Member

nrc commented Feb 1, 2016

Ah, yes that makes sense - we don't mark lint changes as breaking.

@nrc
Copy link
Member

nrc commented Feb 1, 2016

lgtm. @petrochenkov would you mind having a look over this? You're probably way more familiar with the details of resolve than I am.

@bors
Copy link
Contributor

bors commented Feb 1, 2016

☔ The latest upstream changes (presumably #31317) made this pull request unmergeable. Please resolve the merge conflicts.

@jseyfried
Copy link
Contributor Author

Rebased

@bors
Copy link
Contributor

bors commented Feb 3, 2016

☔ The latest upstream changes (presumably #31338) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov
Copy link
Contributor

@nrc

would you mind having a look over this?

All looks reasonable.

@nrc
Copy link
Member

nrc commented Feb 3, 2016

r+ with a rebase

@jseyfried
Copy link
Contributor Author

@nrc rebased

@nrc
Copy link
Member

nrc commented Feb 4, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 4, 2016

📌 Commit 9c166cb has been approved by nrc

@bors
Copy link
Contributor

bors commented Feb 5, 2016

⌛ Testing commit 9c166cb with merge dcf8ef2...

bors added a commit that referenced this pull request Feb 5, 2016
The first commit improves detection of unused imports -- it should have been part of #30325. Right now, the unused import in the changed test would not be reported.

The rest of the commits are miscellaneous, independent clean-ups in resolve that I didn't think warranted individual PRs.

r? @nrc
@bors bors merged commit 9c166cb into rust-lang:master Feb 5, 2016
@jseyfried jseyfried deleted the cleanup branch March 14, 2016 07:08
@jseyfried jseyfried restored the cleanup branch March 14, 2016 07:08
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