Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 pablomartin4btc commented on pull request "doc: Update documentation to include Clang/llvm based coverage report generation":
(https://github.com/bitcoin/bitcoin/pull/31933#discussion_r1994471346)
small nit:
```suggestion
Generating the raw profile data based on `ctest` and functional tests execution:
```
Or remove both and clarify it below before each execution (or any alternative you might have).
🤔 pablomartin4btc reviewed a pull request: "doc: Update documentation to include Clang/llvm based coverage report generation"
(https://github.com/bitcoin/bitcoin/pull/31933#pullrequestreview-2683653107)
ACK 7f57ad00d301b9564ece511d2e99cb0678b21174

I think the feedback about [`CMAKE_BUILD_TYPE`](https://github.com/bitcoin/bitcoin/pull/31933#discussion_r1993037907) needs to be addressed.

Since the functional tests were added (for the report generation), is it worth adding the `fuzz`?
📝 achow101 opened a pull request: "build: Remove manpages when making MacOS app"
(https://github.com/bitcoin/bitcoin/pull/32064)
When creating the MacOS app, the only file that should be in `Bitcoin-Qt.app/Contents/MacOS` is `Bitcoin-Qt. Since #31844, there was also a `share/` containing the manpage for bitcoin-qt. This manpage is not useful to app users, and it is also causing code signing issues. Thus the directory should be removed when making the app.

Fixes https://github.com/bitcoin/bitcoin/issues/32052#issuecomment-2723007926
🚀 fanquake merged a pull request: "depends: remove `NO_HARDEN` option"
(https://github.com/bitcoin/bitcoin/pull/32038)
👍 fanquake approved a pull request: "build: Remove manpages when making MacOS app"
(https://github.com/bitcoin/bitcoin/pull/32064#pullrequestreview-2683941479)
ACK 80b5e7f2cb7fbfbd724e1f52b00c0e72b79a200b
fanquake closed an issue: "v29.0 Testing"
(https://github.com/bitcoin/bitcoin/issues/32052)
🚀 fanquake merged a pull request: "build: Remove manpages when making MacOS app"
(https://github.com/bitcoin/bitcoin/pull/32064)
💬 fanquake commented on pull request "contrib: Fix `gen-bitcoin-conf.sh`":
(https://github.com/bitcoin/bitcoin/pull/32049#issuecomment-2723294891)
Backported in #32062.
⚠️ achow101 reopened an issue: "v29.0 Testing"
(https://github.com/bitcoin/bitcoin/issues/32052)
Umbrella issue for 29.0 testing. Please help testing on a wide variety of supported platforms, as well as interaction with different software.

Let us know which version you tested on which operating system.

If you find an issue, please search Github for known issues first and then open a new Github issue.

This meta issue should not be used to report bugs, as a single thread makes it impossible to track more than one topic.

See [29.0 Release Notes Draft](https://github.com/bitcoin-core/bitcoi
...
💬 Prabhat1308 commented on pull request "contrib: Update coverage.cpp macro to support macOS":
(https://github.com/bitcoin/bitcoin/pull/32059#issuecomment-2723306164)
tACK [611151e](https://github.com/bitcoin/bitcoin/pull/32059/commits/611151eae75ed756437442a5d0356f525c73dd00)
🚀 fanquake merged a pull request: "Require sqlite when building the wallet"
(https://github.com/bitcoin/bitcoin/pull/31961)
💬 ZKcash-IrishGov commented on something "":
(https://github.com/bitcoin/bitcoin/commit/a24419f8bed5e1145ce171dbbdad957750585471#r153704902)
rbmwgtoj4z9npg9nu8wb9q4dejsy5js35oaicefzg3mbf3ncxzu815o7k9ecfagt6ozqp3rxx4g7a3etqajahm4w4j4cyy51sbe4c8zm
💬 fanquake commented on pull request "contrib: Update coverage.cpp macro to support macOS":
(https://github.com/bitcoin/bitcoin/pull/32059#discussion_r1994664138)
Why do we need to specify platforms here, rather than just `__clang__` (especially if you're now going to add fallbacks)?
💬 ajtowns commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#discussion_r1994677857)
Added a bunch of checks to the unit test. The fuzz tester checks at a lower level (`ThresholdConditionCache` vs `VersionBitsCache`) so that's not really suitable.
💬 ajtowns commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#discussion_r1994678038)
Added.
💬 maflcko commented on pull request "contrib: Update coverage.cpp macro to support macOS":
(https://github.com/bitcoin/bitcoin/pull/32059#issuecomment-2723812624)
> In PR [#31901](https://github.com/bitcoin/bitcoin/pull/31901), Coverage.cpp was introduced

For reference the cpp file was created in fa99c3b544b631cfe34d52fb5e71636aedb1b423, but the code was only moved. You can check this by observing it greyed out via `git show fa99c3b544b631cfe34d52fb5e71636aedb1b423 --color-moved=dimmed-zebra`.
💬 maflcko commented on pull request "contrib: Update coverage.cpp macro to support macOS":
(https://github.com/bitcoin/bitcoin/pull/32059#discussion_r1994984199)
in commit "refactor: Improve ResetCoverageCounters function with fallback implem...":

I don't think this is a "refactor" (non-behavior-change), given that CI fails before and passes after.
💬 maflcko commented on pull request "contrib: Update coverage.cpp macro to support macOS":
(https://github.com/bitcoin/bitcoin/pull/32059#discussion_r1994984394)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1994997845)
Thanks and sorry for missing that bash/sh are obviously not used on Windows. I missed the `cmd` section when looking at the link.

Still, it feels a bit uncomfortable that we are using `cmd` on Windows in the CI, which will by default ignore intermediate exit codes.

Given that neither the pull request author, nor any reviewer caught this before someone found it by testing, doesn't increase confidence either.

Thus, I think we should somehow rule out this class of bug, or at least documen
...