Skip to content

code suggestions for unused-mut, while-true, deprecated-attribute, and unused-parens lints#44942

Merged
bors merged 5 commits intorust-lang:masterfrom
zackmdavis:lint_suggestions
Oct 2, 2017
Merged

code suggestions for unused-mut, while-true, deprecated-attribute, and unused-parens lints#44942
bors merged 5 commits intorust-lang:masterfrom
zackmdavis:lint_suggestions

Conversation

@zackmdavis
Copy link
Contributor

@zackmdavis zackmdavis commented Sep 30, 2017

@zackmdavis
fix comment typo, CodeSuggestion path in doc comment
d663003 
`CodeSuggestion` doesn't live in the `diagnostic` module.
estebank
estebank approved these changes Sep 30, 2017
View reviewed changes
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Small nitpicks, r=me once you deal with the first two.

src/librustc_lint/builtin.rs Outdated
Copy link
Contributor

@estebank estebank Sep 30, 2017

Choose a reason for hiding this comment

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

Fits in one line?

src/librustc_lint/builtin.rs Outdated
Copy link
Contributor

@estebank estebank Sep 30, 2017

Choose a reason for hiding this comment

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

When all arguments don't fit, I like to keep one line per argument (indented to the first one in the same line as the method).

src/librustc_lint/unused.rs Outdated
Copy link
Contributor

@estebank estebank Sep 30, 2017

Choose a reason for hiding this comment

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

@nikomatsakis do we care about unicode in rustc comments?

@zackmdavis
Copy link
Contributor Author

zackmdavis commented Sep 30, 2017 •
edited
Loading

(force-pushed to address formatting review comments)

@zackmdavis
Copy link
Contributor Author

zackmdavis commented Sep 30, 2017

(updated again to fix compile-fail failures reported by Travis)

@estebank
Copy link
Contributor

estebank commented Oct 1, 2017

@bors r+

Thanks @zachreizner, great work!

@bors
Copy link
Collaborator

bors commented Oct 1, 2017

📌 Commit a399757 has been approved by estebank

@bors
Copy link
Collaborator

bors commented Oct 1, 2017

⌛ Testing commit a3997577226db861d103d27da91a77acf9fd4b3e with merge 7a521317b909286b51aa36af0dee6cbd7a5cc6c0...

@zackmdavis
Copy link
Contributor Author

zackmdavis commented Oct 1, 2017

Thanks @zachreizner, great work!

I agree; @zachreizner is the best! I especially like the one where he fixed the shift-right docs; it must have taken a very careful eye to catch that!

😉

@zachreizner
Copy link
Contributor

zachreizner commented Oct 1, 2017

Not the first time I've been thanked in place of a a fellow Zach :-). I think my handle just comes up before the Zack Zach-s in the autocomplete.

@bors
Copy link
Collaborator

bors commented Oct 1, 2017

💔 Test failed - status-appveyor

@zackmdavis
Copy link
Contributor Author

zackmdavis commented Oct 1, 2017

Appveyor/Windows apparently doesn't like the --error-format json UI test. I think this is plausibly a bug in the test runner (rather than the JSON order being unstable or anything like that), since the only difference seems to be in the "file_name" value, which is supposed to be normalized (with "$DIR") and worked fine in the Travis/Unix-like tests, but for now I'll just rewrite it as a run-make test.

@estebank
Copy link
Contributor

estebank commented Oct 1, 2017 •
edited
Loading

I believe we're really trying to move away from make tests. @zackmdavis, could you

  1. create a ticket to deal with the test output normalization failure on json output and
  2. revert your last change and add // ignore-windows to the test (this will make the test to be skipped under windows) with a comment linking to the previous ticket?

@zackmdavis zackmdavis mentioned this pull request Oct 2, 2017
Closed
@zackmdavis
Copy link
Contributor Author

zackmdavis commented Oct 2, 2017

output normalization issue is #44968; updated tip of PR with ignore-windows is 979671ec

@zackmdavis
Copy link
Contributor Author

zackmdavis commented Oct 2, 2017

correction, PR is 8a14022

@estebank
Copy link
Contributor

estebank commented Oct 2, 2017

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 2, 2017

📌 Commit 8a14022 has been approved by estebank

@bors
Copy link
Collaborator

bors commented Oct 2, 2017

⌛ Testing commit 8a14022 with merge 9ae6ed7...

bors added a commit that referenced this pull request Oct 2, 2017
@bors
Auto merge of #44942 - zackmdavis:lint_suggestions, r=estebank
9ae6ed7 
code suggestions for unused-mut, while-true, deprecated-attribute, and unused-parens lints

![lint_suggestions](https://user-images.githubusercontent.com/1076988/31044068-b2074de8-a57c-11e7-9319-6668508b6d1f.png)

r? @estebank
@bors
Copy link
Collaborator

bors commented Oct 2, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: estebank
Pushing 9ae6ed7 to master...

@bors bors merged commit 8a14022 into rust-lang:master Oct 2, 2017
This was referenced Oct 11, 2017
Open
Merged
bors added a commit that referenced this pull request Oct 18, 2017
@bors
Auto merge of #45232 - zackmdavis:moar_lint_suggestions, r=estebank
ff33ecb 
code suggestions for non-shorthand field pattern, no-mangle lints

continuing in the spirit of #44942

![moar_lint_suggestions](https://user-images.githubusercontent.com/1076988/31485011-3b20cc80-aee7-11e7-993d-81267ab77732.png)

r? @estebank
bors added a commit that referenced this pull request Oct 18, 2017
@bors
Auto merge of #45232 - zackmdavis:moar_lint_suggestions, r=estebank
395388b 
code suggestions for non-shorthand field pattern, no-mangle lints

continuing in the spirit of #44942

![moar_lint_suggestions](https://user-images.githubusercontent.com/1076988/31485011-3b20cc80-aee7-11e7-993d-81267ab77732.png)

r? @estebank
bors added a commit that referenced this pull request Oct 19, 2017
@bors
Auto merge of #45232 - zackmdavis:moar_lint_suggestions, r=estebank
e3fb84e 
code suggestions for non-shorthand field pattern, no-mangle lints

continuing in the spirit of #44942

![moar_lint_suggestions](https://user-images.githubusercontent.com/1076988/31485011-3b20cc80-aee7-11e7-993d-81267ab77732.png)

r? @estebank
@zackmdavis zackmdavis deleted the lint_suggestions branch January 13, 2018 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estebank estebank estebank approved these changes

Assignees

@estebank estebank

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zackmdavis @estebank @bors @zachreizner