validation: remove unused using directives#25433
validation: remove unused using directives#25433Crypt-iQ wants to merge 1 commit intobitcoin:masterfrom
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
4 participants
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