-
Notifications
You must be signed in to change notification settings - Fork 544
Update lint declaration example #713
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
Conversation
huh, such much speeds 😮 |
impl_lint_pass!( | ||
WhileTrue => [WHILE_TRUE], | ||
); | ||
declare_lint_pass!(WhileTrue => [WHILE_TRUE]); |
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.
We now use declare_lint_pass
macro instead of defining manually. Should we do manually for clarify?
Hm, the link error is unrelated to this, I'll submit another PR not to block the process. |
So, r? @nikomatsakis or @Mark-Simulacrum as per git blame |
Seems fine to me though without the context as to what upstream changes caused this I can't be sure the new code is good. Someone from clippy can likely give better guidance than I. But r=me on the changes presuming they compile. |
I think the issue here is that
@Mark-Simulacrum It'd be great if you could approve this so that "Review required" is satisfied :) |
cc @rust-lang/wg-rustc-dev-guide it'd be great if someone could also review here :) |
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 been a long time since I looked at this code, and it keeps changing a bit over time.
I'm personally fine with merging this and fixing later, if you think that would be the best path forward.
Oh sorry, missed the notification. Feel free to ping me on Zulip in the future, that's much more likely to get through. |
Thanks both!
Gotcha! |
Fixes #712