💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2355903145)
> Should I modify this PR now to be on top of yours, including only the second test? Or would it be better to wait until your PR gets merged and then modify mine?

I would recommend you wait a bit to see what the feedback on that PR is.

🚀 fanquake merged a pull request: "ci: Use macos-14 GHA image (x86_64-apple-darwin22.6.0 -> arm64-apple-darwin23.6.0)"
(https://github.com/bitcoin/bitcoin/pull/30913)

🚀 fanquake merged a pull request: "Remove Autotools packages from CI (and depends doc)"
(https://github.com/bitcoin/bitcoin/pull/30902)

🚀 fanquake merged a pull request: "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake"
(https://github.com/bitcoin/bitcoin/pull/30888)

🚀 fanquake merged a pull request: "doc: update NeedsRedownload() and nStatus comment"
(https://github.com/bitcoin/bitcoin/pull/29624)

👍 hodlinator approved a pull request: "log: Use ConstevalFormatString"
(https://github.com/bitcoin/bitcoin/pull/30889#pullrequestreview-2309847632)
ACK fabd78e2e30e557e04739e29c9c221ad47245df1

Concept: Great to have compile time format string error checking on the common logging functions!

Tested changing this in *logging.h*:
```diff
- log_msg = tfm::format(fmt, params...);
+ log_msg = tfm::format("%s %s\n", 1.1f);
```
and ran with expected error output.

Agree with ryanofsky that it would be good to document rough compile time impact if any.

💬 hodlinator commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#discussion_r1763327444)
nit: Line grows from 209 to 253 chars, please consider inserting line break(s).
```suggestion
inline void LogPrintFormatInternal(std::string_view logging_function, std::string_view source_file, const int source_line,
const BCLog::LogFlags flag, const BCLog::Level level, util::ConstevalFormatString<sizeof...(Params)> fmt, const Params&... params)
```

💬 hodlinator commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#discussion_r1763307360)
nit: The `Params` name was already used in *scriptpubkeyman.h* and *wallet.h* before the PR but `Args` is used in *util/string.h*, *index/base.h/cpp*, *netbase.cpp* and used to be used here. Why switch and touch 2 additional lines?

The old way feels more consistent with the C "varargs". But maybe there's a convincing argument for "Params".

Did the change locally and it compiles without causing any non-obvious issue.

👋 hebasto's pull request is ready for review: "ci: Use `ninja` to build in macOS native CI job"
(https://github.com/bitcoin/bitcoin/pull/30915)

💬 hebasto commented on pull request "ci: Use `ninja` to build in macOS native CI job":
(https://github.com/bitcoin/bitcoin/pull/30915#issuecomment-2356083087)
cc @maflcko

💬 hebasto commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2356089545)
@theStack

Could rebase once more please to refresh the CI?

💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2356103199)
> Is this so that re-runs (of the same PR) always hit the cache? If that's it I'm not _totally_ sure it's worth it. My intuition says that if we had multiple servers, even if a re-run took place on a different machine, we'd likely get a _pretty decent_ cache hit from another run or a previous master build

Right. It is unclear whether this is worth it. I think when it comes to builds on master, the speed doesn't matter at all, because no one is waiting for the result to appear immediately. So
...

💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2356181083)
> CPU-bound (and not IO-bound)

Actually, the long-running fuzz test may be IO-bound so using a ramdisk could speed them up. So this would be another idea to look at.

💬 theStack commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2356184027)
> @theStack
>
> Could rebase once more please to refresh the CI?

Yes, done.

👍 hebasto approved a pull request: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827#pullrequestreview-2310065055)
ACK 06a7df70df30879e0b691d1a252636f703b8cdfb, I've backported the listed PRS locally. The only merge conflict I faced was in https://github.com/bitcoin/bitcoin/pull/30807. It was trivial to resolve.

💬 maflcko commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2356197180)
> It's clear to see that the header generation is self-contained, and afaik that's the only place that would matter for build performance.

Could rebase on current master, now that the header generation is 30x faster, after 2a0949f0977db2dbb7ac452e8d58d413b9526756 ? This should also change the benchmark in the pull description.

💬 maflcko commented on pull request "ci: Use `ninja` to build in macOS native CI job":
(https://github.com/bitcoin/bitcoin/pull/30915#issuecomment-2356214347)
review ACK d01b85bfecbcf16ea16f90e2ade7537bf582269f

Can't hurt to test this in one task

👍 ryanofsky approved a pull request: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block"
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2310085862)
Code review ACK 7942951e3fcc58f7db0059546d03be9ea243f1db. Just rebased since last review

👍 TheCharlatan approved a pull request: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block"
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2310089065)
Re-ACK 7942951e3fcc58f7db0059546d03be9ea243f1db

💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#issuecomment-2356233258)
Rebased after merge of #30286.