Skip to content

Conversation

@kraai
Copy link
Contributor

@kraai kraai commented Apr 15, 2019

No description provided.

@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 16, 2019
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

There are some files untouched, e.g. clippy_lints/src/unwrap.rs and clippy_lints/src/ptr_offset_with_cast.rs. Others? *)

Some things I noticed by going through these changes, that need to be addressed in one way or another.


*) LintPass is still implemented by hand for this 2 files above. For both files this is not necessary. I assume you missed those by searching for impl LintPass with a regexp. In clippy_lints/src/ptr_offset_with_cast.rs the imports should be fixed.

@kraai
Copy link
Contributor Author

kraai commented Apr 16, 2019

There are some files untouched, e.g. clippy_lints/src/unwrap.rs and clippy_lints/src/ptr_offset_with_cast.rs. Others? *)

Some things I noticed by going through these changes, that need to be addressed in one way or another.

*) LintPass is still implemented by hand for this 2 files above. For both files this is not necessary. I assume you missed those by searching for impl LintPass with a regexp. In clippy_lints/src/ptr_offset_with_cast.rs the imports should be fixed.

Oops, I searched for impl LintPass so I didn't catch those. Thanks for finding them.

@kraai kraai force-pushed the lint-pass-macros branch from 23c8408 to 1b8f031 Compare April 16, 2019 17:08

#[derive(Copy, Clone)]
pub struct Pass;
declare_lint_pass!(NeedlessUpdate => [NEEDLESS_UPDATE]);
Copy link
Member

Choose a reason for hiding this comment

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

cc #3968 (comment)

It seems that it doesn't trigger the module_name_repetition lint

@flip1995
Copy link
Member

This LGTM now! Thanks for the quick fixes.

Could you extend the test file tests/ui/lint_without_lint_pass.rs by adding 2 LintPass definitions with those macros (declare/impl_lint_pass)?

@bors
Copy link
Contributor

bors commented Apr 17, 2019

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

@flip1995
Copy link
Member

Thanks! Just needs a rebase.

@kraai
Copy link
Contributor Author

kraai commented Apr 17, 2019

@flip1995 I'm trying to rebase it now.

@kraai kraai force-pushed the lint-pass-macros branch from a53c108 to ef29db7 Compare April 17, 2019 16:35
@flip1995
Copy link
Member

Thanks!

@bors r+ p=1

@bors
Copy link
Contributor

bors commented Apr 18, 2019

📌 Commit ef29db7 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Apr 18, 2019

⌛ Testing commit ef29db7 with merge 58e1213...

bors added a commit that referenced this pull request Apr 18, 2019
@bors
Copy link
Contributor

bors commented Apr 18, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing 58e1213 to master...

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

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants