💬 fanquake commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121666498)
> to avoid issues
What issues? Do I understand correctly, that if we land the patch in the subtree, this will just break again?
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121666498)
> to avoid issues
What issues? Do I understand correctly, that if we land the patch in the subtree, this will just break again?
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121670336)
The following `git diff` prints this patch even IWYU did not apply any modifications.
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121670336)
The following `git diff` prints this patch even IWYU did not apply any modifications.
💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2121670967)
Wow my bad.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2121670967)
Wow my bad.
💬 fanquake commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121672661)
?
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121672661)
?
💬 fanquake commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121672770)
?
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121672770)
?
📝 fanquake opened a pull request: "depends: Bump boost to 1.88.0 and use new CMake buildsystem"
(https://github.com/bitcoin/bitcoin/pull/32665)
Originally #30434.
This has a few advantages over the old method of simply copying headers:
- Installs proper CMake files which can be picked up by our buildsystem
- Only installs necessary headers, not all of Boost
Pulls in upstreamed https://github.com/boostorg/test/pull/445.
(https://github.com/bitcoin/bitcoin/pull/32665)
Originally #30434.
This has a few advantages over the old method of simply copying headers:
- Installs proper CMake files which can be picked up by our buildsystem
- Only installs necessary headers, not all of Boost
Pulls in upstreamed https://github.com/boostorg/test/pull/445.
✅ fanquake closed a pull request: "depends: bump boost to 1.87.0 and use new CMake buildsystem"
(https://github.com/bitcoin/bitcoin/pull/30434)
(https://github.com/bitcoin/bitcoin/pull/30434)
💬 fanquake commented on pull request "depends: bump boost to 1.87.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/30434#issuecomment-2931555985)
I've pulled @hebasto's newer branch into #32665.
(https://github.com/bitcoin/bitcoin/pull/30434#issuecomment-2931555985)
I've pulled @hebasto's newer branch into #32665.
💬 fanquake commented on pull request "uint256 cxx-20 constexpr patch":
(https://github.com/bitcoin/bitcoin/pull/32663#issuecomment-2931558582)
Was this opened by accident?
(https://github.com/bitcoin/bitcoin/pull/32663#issuecomment-2931558582)
Was this opened by accident?
💬 hebasto commented on pull request "depends: Bump boost to 1.88.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/32665#issuecomment-2931563110)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/32665#issuecomment-2931563110)
Concept ACK.
💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2121686616)
Done.
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2121686616)
Done.
💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2121695775)
What do you think about using the "exposed `std::unique_ptr<>`" pattern (don't know if it has a name) as opposed to full-blown pimpl.
So like `TxGraph` / `TxGraphImpl`, I'm suggesting there would be 2 files (no `_impl.h`), with a `TxOrphanage` abstract class with virtual member functions, and an implementation defined only inside the `.cpp` file, and a factory function `std::unique_ptr<TxOrphanage> MakeTxOrphanage()` that returns a newly-constructed `TxOrphanageImpl`. It avoids the boilerplat
...
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2121695775)
What do you think about using the "exposed `std::unique_ptr<>`" pattern (don't know if it has a name) as opposed to full-blown pimpl.
So like `TxGraph` / `TxGraphImpl`, I'm suggesting there would be 2 files (no `_impl.h`), with a `TxOrphanage` abstract class with virtual member functions, and an implementation defined only inside the `.cpp` file, and a factory function `std::unique_ptr<TxOrphanage> MakeTxOrphanage()` that returns a newly-constructed `TxOrphanageImpl`. It avoids the boilerplat
...
📝 hebasto converted_to_draft a pull request: "ci, iwyu: Treat warnings as errors for specific directories"
(https://github.com/bitcoin/bitcoin/pull/31308)
This PR is the first step towards treating IWYU warnings as errors. At this stage, it applies only to the `crypto` and `index` directories.
(https://github.com/bitcoin/bitcoin/pull/31308)
This PR is the first step towards treating IWYU warnings as errors. At this stage, it applies only to the `crypto` and `index` directories.
👋 hebasto's pull request is ready for review: "ci, iwyu: Treat warnings as errors for specific directories"
(https://github.com/bitcoin/bitcoin/pull/31308)
(https://github.com/bitcoin/bitcoin/pull/31308)
🤔 jonatack reviewed a pull request: "Fuzz: extend CConnman tests"
(https://github.com/bitcoin/bitcoin/pull/28584#pullrequestreview-2889257494)
re-ACK 582d9e3dbdf5b64272b65844d679942c5aca643f per `git range-diff 1c66023 c97d496 582d9e3`
changes since my last review https://github.com/bitcoin/bitcoin/pull/28584#pullrequestreview-2446770519 have been rebases and adding commit cf0a49723bbbbb22d47ccb8f0e030a51e6757609
(https://github.com/bitcoin/bitcoin/pull/28584#pullrequestreview-2889257494)
re-ACK 582d9e3dbdf5b64272b65844d679942c5aca643f per `git range-diff 1c66023 c97d496 582d9e3`
changes since my last review https://github.com/bitcoin/bitcoin/pull/28584#pullrequestreview-2446770519 have been rebases and adding commit cf0a49723bbbbb22d47ccb8f0e030a51e6757609
💬 fanquake commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#issuecomment-2931604386)
https://cirrus-ci.com/task/5352790488252416?logs=ci#L1675:
```bash
[09:31:48.288] Extracting freetype...
[09:31:48.316] /ci_container_base/depends/sources/freetype-2.11.0.tar.xz: OK
[09:31:48.562] Preprocessing freetype...
[09:31:48.575] Configuring freetype...
[09:31:48.656] CMake Error at CMakeLists.txt:100 (cmake_minimum_required):
[09:31:48.656] Compatibility with CMake < 3.5 has been removed from CMake.
[09:31:48.656]
[09:31:48.656] Update the VERSION argument <min> value. Or
...
(https://github.com/bitcoin/bitcoin/pull/32595#issuecomment-2931604386)
https://cirrus-ci.com/task/5352790488252416?logs=ci#L1675:
```bash
[09:31:48.288] Extracting freetype...
[09:31:48.316] /ci_container_base/depends/sources/freetype-2.11.0.tar.xz: OK
[09:31:48.562] Preprocessing freetype...
[09:31:48.575] Configuring freetype...
[09:31:48.656] CMake Error at CMakeLists.txt:100 (cmake_minimum_required):
[09:31:48.656] Compatibility with CMake < 3.5 has been removed from CMake.
[09:31:48.656]
[09:31:48.656] Update the VERSION argument <min> value. Or
...
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121717145)
Thanks! Reworked.
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121717145)
Thanks! Reworked.
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121717600)
Sure! Fixed.
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121717600)
Sure! Fixed.
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121718094)
Thanks! Fixed.
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121718094)
Thanks! Fixed.
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121718259)
Thanks! Fixed.
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121718259)
Thanks! Fixed.