💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756411531)
> > The verbosity in tests is a bit annoying, but seems acceptable to me.
>
> I agree.

I pushed a test-only change in the latest push to reduce the verbosity to a minimum.

👍 fanquake approved a pull request: "ci: Post CMake-migration fixes and amendments"
(https://github.com/bitcoin/bitcoin/pull/30841#pullrequestreview-2299649592)
ACK c45186ca548362b75a6640393ccf79b11ff727da

🚀 fanquake merged a pull request: "ci: Post CMake-migration fixes and amendments"
(https://github.com/bitcoin/bitcoin/pull/30841)

💬 0xB10C commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2345677280)
I recently came across @aureleoules's https://corecheck.dev/benchmarks which tracks execution time, CPU instructions, and CPU cycles of the Bitcoin Core benchmarks.

💬 maflcko commented on issue "GUI event loop should be block free":
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2345680377)
What is the status of the QML GUI? IIRC it was put on hold, because the Android APK in depends couldn't be updated for some reason, but I may be misremembering.

💬 hodlinator commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756443830)
<details>
<summary>

#### Diff on top of fae8c25d07cf003df1470699df4ef475055bb885

</summary>

```diff
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
index 4c61632fce..ed383e9dad 100644
--- a/src/test/util_string_tests.cpp
+++ b/src/test/util_string_tests.cpp
@@ -17,7 +17,53 @@ template <unsigned NumArgs>
inline void PassFmt(util::ConstevalFormatString<NumArgs> fmt)
{
decltype(fmt)::Detail_CheckNumFormatSpecifiers(fmt.fmt); // This was run at c
...

👍 fanquake approved a pull request: "build: Use CMake's default permissions in macOS `deploy` target"
(https://github.com/bitcoin/bitcoin/pull/30838#pullrequestreview-2299668588)
ACK 5ba03e7d35e1b47ba864c9ae3c94af97cd3ae10b - I'm going to merge this now so we return to usable (comparable) guix builds.

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756448598)
> Still dropping a few negative tests, since they are proven by positive tests, even if less straightforward without the free function.

I don't think this is true. Without the free function, the tests are required to catch bugs, such as removing a check. E.g. see this diff that removes a check and the tests that you want to remove:

```diff
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
index 4c61632fce..42facd4dc1 100644
--- a/src/test/util_string_tests.cpp
...

💬 hodlinator commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756457285)
> > Still dropping a few negative tests, since they are proven by positive tests, even if less straightforward without the free function.
>
> I don't think this is true. Without the free function

True, forgot that that last condition was back in the non-free function. Negative tests are not removed in [latest diff](https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756443830).

💬 maflcko commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2345700640)
Yeah, corecheck is nice, but I think it went down after `cmake`. See also https://github.com/corecheck/corecheck/issues/10

💬 maflcko commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2345720633)
Another thing to add (back) to the benchmarks would be compile memory usage, which is easy to achieve with `$(which time) -f "%M;%S;%U" ...`, c.f. https://github.com/chaincodelabs/bitcoinperf/blob/0a1d6f54263cca78e645544ab8247d106d5ff59c/runner/sh.py#L110

✅ fanquake closed an issue: "build: reproducibility issue with macOS Guix builds"
(https://github.com/bitcoin/bitcoin/issues/30815)

🚀 fanquake merged a pull request: "build: Use CMake's default permissions in macOS `deploy` target"
(https://github.com/bitcoin/bitcoin/pull/30838)

💬 hodlinator commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#issuecomment-2345733518)
https://github.com/bitcoin/bitcoin/pull/30328#pullrequestreview-2201704324 still applies to 705d9eb0aef8831891e1cce80c33615440547e90 instead of d5d994c02bb54db395da457724ec45539f1c10a8.

(Clarified linked comment a bit).

⚠️ fanquake opened an issue: "cmake: adjust Find modules to try `find_package` & fallback to `pkg_check_modules`"
(https://github.com/bitcoin/bitcoin/issues/30876)
See the discussion in: https://github.com/bitcoin/bitcoin/pull/30803#discussion_r1743507523.

> I assume one thing we could do, is try find_package(ZeroMQ) first, and fallback to pkg_check_modules(libzmq), and just maintain this (pretty much) forever. That might be better than somewhat vauge comments about "modern" distros, because, (some) modern distros do already support this, and it's actually the case that all versions of all distros Bitcoin Core supports need to do this, before we could d
...

💬 fanquake commented on pull request "build: Minor build system fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30803#discussion_r1756492407)
Opened an issue for this #30876.

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756495304)
Thanks, the diff looks good, but would you mind if you submitted it in a follow-up? I presume you ran it locally, and it didn't uncover any bugs in this pull request, so that is a useful signal already. However, at some point I want to wrap up with this pull request myself. There are already 150 comments, most of which are hidden by GitHub, and thus almost impossible to navigate.

If I included your diff, someone may suggest to "reduce verbosity" by using the compiler to construct the args of
...

🚀 fanquake merged a pull request: "build: Minor build system fixes and amendments"
(https://github.com/bitcoin/bitcoin/pull/30803)

🤔 l0rinc requested changes to a pull request: "util: Use consteval checked format string in FatalErrorf, LogConnectFailure"
(https://github.com/bitcoin/bitcoin/pull/30546#pullrequestreview-2299632852)
It seems to me we still have a remaining trailing formatter bug that we should fix before pushing

💬 l0rinc commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756421832)
Some tense changes could clarify the exact behavior:
```suggestion
decltype(fmt)::Detail_CheckNumFormatSpecifiers(fmt.fmt); // This was already executed at compile-time, but is executed again at run-time to avoid -Wunused.
```