I noticed the following were unused from the node namespace in validation.cpp:
- BLOCKFILE_CHUNK_SIZE
- nPruneTarget
- OpenBlockFile
- UNDOFILE_CHUNK_SIZE
I am not sure if maybe there is some reason these are still defined here in which case I'll close this
validation: remove unused using directives #25433
pull Crypt-iQ wants to merge 1 commits into bitcoin:master from Crypt-iQ:remove_using changing 1 files +0 −4-
Crypt-iQ commented at 11:58 PM on June 20, 2022: contributor
-
73271802ae
validation: remove unused using directives
The following were unused from the node namespace: - BLOCKFILE_CHUNK_SIZE - nPruneTarget - OpenBlockFile - UNDOFILE_CHUNK_SIZE
- DrahtBot added the label Validation on Jun 21, 2022
-
DrahtBot commented at 4:48 AM on June 21, 2022: member
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Conflicts
No conflicts as of last run.
- laanwj added the label Refactoring on Jun 21, 2022
- laanwj removed the label Validation on Jun 21, 2022
-
Crypt-iQ commented at 10:29 PM on June 22, 2022: contributor
I noticed there is another one in
src/rpc/transaction.cppforReadBlockFromDisk -
MarcoFalke commented at 5:27 AM on June 23, 2022: member
not sure if this is worth it. If it is, maybe a iwyu/linter/tidy check would be better?
-
Crypt-iQ commented at 12:08 PM on June 24, 2022: contributor
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
-
MarcoFalke commented at 12:39 PM on June 24, 2022: member
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:
- pr flood to "fix" this issue
- review comments to suggest to "fix" this issue
- wasted review on re-ACKs if the style comment is addressed
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.)
- Crypt-iQ closed this on Jun 24, 2022
- MarcoFalke referenced this in commit 47c86a023d on Jul 19, 2022
- PastaPastaPasta referenced this in commit b6c18ae577 on Oct 18, 2022
- DrahtBot locked this on Jun 24, 2023
Contributors
Labels