No description provided.
Consensus: Remove unused dependencies #12672
pull shmulim wants to merge 1 commits into bitcoin:master from shmulim:removed-unused-deps changing 1 files +0 −2-
shmulim commented at 10:33 PM on March 11, 2018: none
-
Remove unused dependencies d7549e4587
- MarcoFalke added the label Refactoring on Mar 11, 2018
-
MarcoFalke commented at 10:36 PM on March 11, 2018: member
Is this a complete fix for all files in the repository? Also, steps to reproduce would be nice. Did you use
iwyu? -
shmulim commented at 11:14 PM on March 11, 2018: none
This pertains only to
params.h, and does not represent a comprehensive check for the possibility of other unused dependencies. No, I did not runiwyu, only code-review. -
MarcoFalke commented at 11:46 PM on March 11, 2018: member
Tend to NACK in that case. Otherwise this will generate a flood of pull requests for each affected file. This probably would also need a travis check.
-
fanquake commented at 12:59 AM on March 12, 2018: member
I agree with @MarcoFalke . Refactoring like this can't be done one file at a time, as it's noisy, and will result in hundreds of PRs. It's also hard to do one large (hundreds of files at once) PR, as it will mean rebasing everything else. Also, as long as duplicate/un-needed includes checks aren't enforced before anything is merged, these will continually creep into the code.
- fanquake closed this on Mar 12, 2018
- DrahtBot locked this on Sep 8, 2021
Contributors
Labels