Skip to content

Lint category macro-ception #6830

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

Closed
wants to merge 1 commit into from
Closed

Conversation

camsteffen
Copy link
Contributor

changelog: none

Caution: The speed of time reduces by a factor of two with each nested macro invocation.

@rust-highfive
Copy link

r? @Manishearth

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 2, 2021
@flip1995
Copy link
Member

flip1995 commented Mar 3, 2021

Hmm.. I don't really like this. It makes this simple piece of code so much more complex. I think the code duplication is fine here, since this is a part of code that almost never has to be touched. So I prefer the simplicity over DRY in this case.

@camsteffen
Copy link
Contributor Author

camsteffen commented Mar 3, 2021

I know what you mean. I could make it a tad simpler by inlining the categories macro?

@camsteffen
Copy link
Contributor Author

I changed it. But feel free to close.

@Manishearth
Copy link
Member

Yeah I kinda agree with flip here. The trick is cool, but I think readability matters more than DRY here

@camsteffen camsteffen closed this Mar 3, 2021
xFrednet added a commit to xFrednet/rust-clippy that referenced this pull request Feb 13, 2022
This is a revival of rust-lang#6830. It was closed to keep the currently simple implementation. However, with the upcoming changes it's better to follow the DRY princliple.

Co-authored-by: camsteffen <[email protected]>
xFrednet added a commit to xFrednet/rust-clippy that referenced this pull request Feb 14, 2022
This change was initially suggested by `@camsteffen` in rust-lang#6830. Back then, it was decided to avoid this change for simplicity. Now, I think it's reasonable to have it to avoid several repetitions for nightly lints.

Co-authored-by: camsteffen [email protected]
xFrednet added a commit to xFrednet/rust-clippy that referenced this pull request Feb 15, 2022
This was first suggested by `@camsteffen` in rust-lang#6830. Back then, it was decided to avoid this implementation to keep the macro simple. Now it makes sense to again use this macro, as the implementation of the macro will get a bit more complicated to support nightly lints.

Credit where credit is due:

Co-authored-by: camsteffen <[email protected]>
xFrednet added a commit to xFrednet/rust-clippy that referenced this pull request Feb 17, 2022
This was first suggested by `@camsteffen` in rust-lang#6830. Back then, it was decided to avoid this implementation to keep the macro simple. Now it makes sense to again use this macro, as the implementation of the macro will get a bit more complicated to support nightly lints.

Credit where credit is due:

Co-authored-by: camsteffen <[email protected]>
xFrednet added a commit to xFrednet/rust-clippy that referenced this pull request Feb 17, 2022
This was first suggested by `@camsteffen` in rust-lang#6830. Back then, it was decided to avoid this implementation to keep the macro simple. Now it makes sense to again use this macro, as the implementation of the macro will get a bit more complicated to support nightly lints.

Credit where credit is due:

Co-authored-by: camsteffen <[email protected]>
xFrednet added a commit to xFrednet/rust-clippy that referenced this pull request Feb 17, 2022
This was first suggested by `@camsteffen` in rust-lang#6830. Back then, it was decided to avoid this implementation to keep the macro simple. Now it makes sense to again use this macro, as the implementation of the macro will get a bit more complicated to support nightly lints.

Credit where credit is due:

Co-authored-by: camsteffen <[email protected]>
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.

4 participants