Skip to content

refactor: add most of src/util to iwyu#25324

Merged
maflcko merged 1 commit into
bitcoin:masterfrom
fanquake:src_util_iwyu
Jul 12, 2022
Merged

refactor: add most of src/util to iwyu#25324
maflcko merged 1 commit into
bitcoin:masterfrom
fanquake:src_util_iwyu

Conversation

@fanquake

@fanquake fanquake commented Jun 9, 2022

Copy link
Copy Markdown
Member

These files change infrequently, and not much header shuffling is required.

We don't add everything in src/util/ yet, because IWYU makes some
dubious suggestions, which I'm going to follow up with upstream.

Soon we'll swap src/util/xyz.cpp for just src/util/.

@hebasto

hebasto commented Jun 9, 2022

Copy link
Copy Markdown
Member

Concept ACK.

@DrahtBot

DrahtBot commented Jun 9, 2022

Copy link
Copy Markdown
Contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #25112 (util: Move error message formatting of NonFatalCheckError to cpp by MarcoFalke)
  • #24058 (BIP-322 basic support by kallewoof)

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.

Comment thread ci/test/06_script_b.sh Outdated
Comment on lines 49 to 61

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, this PR affects more files, e.g., asmap.cpp, readwritefile.cpp etc. Should they be listed here as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

IWYU generates spurious output for some .cpp files, which would require adding headers that I'm not sure are correct, so I've excluded them for now.

These files change infrequently, and not much header shuffling is required.

We don't add everything in src/util/ yet, because IWYU makes some
dubious suggestions, which I'm going to follow up with upstream.

@hebasto hebasto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK 07f2c25, I have reviewed the code and it looks OK, I agree it can be merged.

Suggesting to apply clang-format-diff.py as well.

@maflcko
maflcko merged commit 1d89fc6 into bitcoin:master Jul 12, 2022
@fanquake
fanquake deleted the src_util_iwyu branch July 12, 2022 19:16
@bitcoin bitcoin locked and limited conversation to collaborators Sep 7, 2023
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.

4 participants