8000 refactor: Fix timedata includes by maflcko · Pull Request #29361 · bitcoin/bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

refactor: Fix timedata includes #29361

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

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

maflcko
Copy link
Member
@maflcko maflcko commented Feb 1, 2024

Remove unused includes. Also, fixup comments, see https://github.com/bitcoin/bitcoin/pull/28956/files#r1464827885. Also, add missing includes to chain.h while touching it.

@DrahtBot
Copy link
Contributor
DrahtBot commented Feb 1, 2024

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK dergoegge, stickies-v, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #27006 (reduce cs_main scope, guard block index 'nFile' under a local mutex by furszy)

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.

Copy link
Contributor
@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK, but I think there are a couple more include changes that should be made as per my diff in #28956 (review)

Copy link
Member
@dergoegge dergoegge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK fad0faf

@DrahtBot DrahtBot requested a review from stickies-v February 2, 2024 09:49
Copy link
Contributor
@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK fad0faf

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: iwyu suggests

chain.h should add these lines:
namespace Consensus { struct Params; }

chain.h should remove these lines:
- #include <consensus/params.h>  // lines 10-10

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to leave removal of further includes to a follow-up

@achow101
Copy link
Member
achow101 commented Feb 2, 2024

ACK fad0faf

@achow101 achow101 merged commit 3894104 into bitcoin:master Feb 2, 2024
@maflcko maflcko deleted the 2402-timedata-includes- branch February 3, 2024 09:08
@bitcoin bitcoin locked and limited conversation to collaborators Feb 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0