Remove now-unnecessary poll, fcntl includes from net(base).cpp #27530
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
Can you add this to iwyu CI to double check and aid reviewers? |
|
tACK I also ran netbase.cpp through the iwyu CI and seems like there maybe a few more unused imports, but I haven't thoroughly tested removing the other ones
|
I did that recently for these files while working on #27385; the results are here: https://cirrus-ci.com/task/6749578737745920 |
There was a problem hiding this comment.
utACK b095ac5
Noticed these while doing #27385. Per https://cirrus-ci.com/task/6749578737745920:
net.cpp should remove these lines:
- #include <clientversion.h> // lines 15-15
- #include <crypto/sha256.h> // lines 18-18
- #include <fcntl.h> // lines 45-45
- #include <math.h> // lines 63-63
- #include <poll.h> // lines 53-53
- #include <array> // lines 57-57
- #include <unordered_map> // lines 61-61
netbase.cpp should remove these lines:
- #include <fcntl.h> // lines 26-26
- #include <poll.h> // lines 30-30
- #include <chrono> // lines 19-19
- #include <cstdint> // lines 20-20
- #include <limits> // lines 22-22
Edit: #include <fcntl.h> should be added to util/sock.cpp
|
I'll update this PR on Sunday, just busy with http://btcpp.dev at the moment. |
158cc33 to
26bdf2a
Compare
f0e7d5b to
1065bbc
Compare
|
The scope of this has expanded, and it now conflicts with a number of things, that are much higher priority. You could wait until all the kernel / assumeutxo changes are merged, (and maybe also 27571, to remove the need to update the list), or, revert back to the original non-conflicting change, and that could probably be merged. Otherwise, I think this is likely to just sit for some time, and mostly generate merge-conflict noise in other drahtbot/other PRs. |
As far as I can tell, the code calling for these includes was removed in: 6e68ccb bitcoin#24356 82d360b bitcoin#21387
e6b6efc to
8d9b90a
Compare
… net(base).cpp 8d9b90a Remove now-unnecessary poll, fcntl includes from net(base).cpp (Ben Woosley) Pull request description: As far as I can tell, the code calling for these includes was removed in: 6e68ccb bitcoin#24356 82d360b bitcoin#21387 ACKs for top commit: fanquake: ACK 8d9b90a Tree-SHA512: e536d60f4cf204a10a5b461eca20c8329aa6b0fd3b27651ac9490ed42d3e22e31d652cd991ddc84c96e4758df15aefa7e7f84c710f2feb6d2e0fcfbda9ad4356
As far as I can tell, the code calling for these includes was removed in:
6e68ccb #24356
82d360b #21387