-
Notifications
You must be signed in to change notification settings - Fork 38.2k
validation: remove unused using directives #25433
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
The following were unused from the node namespace: - BLOCKFILE_CHUNK_SIZE - nPruneTarget - OpenBlockFile - UNDOFILE_CHUNK_SIZE
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
|
I noticed there is another one in |
|
not sure if this is worth it. If it is, maybe a iwyu/linter/tidy check would be better? |
|
I think an automated check would be better than this, but I wouldn't want to add something that rarely catches anything (I think an unused using directive would be kind of rare?). Maybe it's better to let somebody remove these as part of a larger validation PR |
|
We already run clang-tidy, so there is no additional cost of enabling https://clang.llvm.org/extra/clang-tidy/checks/misc/unused-using-decls.html The question is: Is it worth it? If the clang-tidy check helps to avoid:
I'd say it is worth it. (Surely it is frustrating to open a pull request and see a red CI 6 minutes later, but if a pull request author isn't willing to put up 6 minutes of their time, maybe they shouldn't open a pull request in the first place to ask others to spend time on it.) |
a02f3f1 tidy: use misc-unused-using-decls (fanquake) d6787bc refactor: remove unused using directives (fanquake) 3617634 validation: remove unused using directives (eugene) Pull request description: Adds https://clang.llvm.org/extra/clang-tidy/checks/misc/unused-using-decls.html to our clang-tidy. PR'd after the discussion in #25433 (which it includes). ACKs for top commit: jamesob: Github ACK a02f3f1 Tree-SHA512: 2bb937c1cc90006e69054458d845fb54f287567f4309c773a3fc859f260558c32ff51fc1c2ce9b43207426f3547e7ce226c87186103d741d5efcca19cd355253
a02f3f1 tidy: use misc-unused-using-decls (fanquake) d6787bc refactor: remove unused using directives (fanquake) 3617634 validation: remove unused using directives (eugene) Pull request description: Adds https://clang.llvm.org/extra/clang-tidy/checks/misc/unused-using-decls.html to our clang-tidy. PR'd after the discussion in bitcoin#25433 (which it includes). ACKs for top commit: jamesob: Github ACK bitcoin@a02f3f1 Tree-SHA512: 2bb937c1cc90006e69054458d845fb54f287567f4309c773a3fc859f260558c32ff51fc1c2ce9b43207426f3547e7ce226c87186103d741d5efcca19cd355253
I noticed the following were unused from the node namespace in validation.cpp:
I am not sure if maybe there is some reason these are still defined here in which case I'll close this