💬 hodlinator commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1768357254)
Done in 2955b1a1f3eec934960e880963a09b359d828721.

💬 l0rinc commented on pull request "doc: Add `nproc` support for Mac through `coreutils`":
(https://github.com/bitcoin/bitcoin/pull/30936#issuecomment-2363381454)
Thanks for the comments, would it make sense to add this to the mac developer notes instead, or should we remove the `-j N` docs, since people are migrating to Ninja anyway?
This stupid `nproc` problem comes up often for macs, was hoping we can finally get rid of it...

📝 fanquake unlocked a pull request: "refactor, kernel: Remove gArgs accesses from dbwrapper and txdb"
(https://github.com/bitcoin/bitcoin/pull/25862)
Code in the libbitcoin_kernel library should not be calling `ArgsManager` methods or trying to read options from the command line. Instead it should just get options values from simple structs and function arguments that are passed in externally. This PR removes `gArgs` accesses from `dbwrapper` and `txdb` modules by defining appropriate options structs, and is a followup to PR's #25290 #25487 #25527 which remove other `ArgsManager` calls from kernel modules.

This PR does not change behavior
...

💬 maflcko commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1768368784)
Yes, exactly. Depending on how https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1767054547 turns out, this test-only pull will have to be adjusted.

There are already 4 different follow-ups, all of which explicitly or implicitly conflict with each other.

I don't really want to open a 5th one to remove the linter first.

The more pull requests exist that explicitly or implicitly conflict with each other, the more review will be wasted, because if one of them is merged, all others
...

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2363412596)
Looks like this is still a problem. The macOS 14 task was able to finish in less than 30 minutes, even without a ccache at all.

Now it is stuck again for more than 90 minutes: https://github.com/bitcoin/bitcoin/actions/runs/10956545011/job/30422825062?pr=30935#step:7:3249

🤔 stickies-v reviewed a pull request: "log: Enforce trailing newline at compile time"
(https://github.com/bitcoin/bitcoin/pull/30929#pullrequestreview-2317918165)
Concept ACK on these checks being done at compile-time instead of through clang-tidy/linters/..., and the significant resulting code clean-up. Added verbosity can be hidden, e.g. by the alias suggested as a nit.

💬 stickies-v commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1768397353)
nit: might be nice introducing a `LogFmt` alias here?

<details>
<summary>git diff on fa7190db83</summary>

```diff
diff --git a/src/logging.h b/src/logging.h
index e0bd79f793..ed224565e6 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -239,7 +239,10 @@ static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level leve
bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);

template <typename... Args>
-inline void LogPrintFormatInternal(std::string_vie
...

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2363454663)
Fixed ci

📝 theStack opened a pull request: "scripted-diff: drop config/ subdir for bitcoin-config.h"
(https://github.com/bitcoin/bitcoin/pull/30937)
This PR is a follow-up to #30856, as suggested in comment https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2356804690. With the scripted diff, review should be fairly trivial, but it could still be seen as controversial due to the large number of files (78 in total) being touched.

💬 ismaelsadeeq commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#discussion_r1768427965)
nit
```suggestion
self.log.info("Test loading backup on a pruned node when the backup was created close to the prune height of the restoring node")

```

💬 ismaelsadeeq commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#discussion_r1768424398)
nit:
Assert that the chain height is 214, because the test assume so?
```suggestion
# Ensure the chain tip is at height 214, because this test assume it is.
assert_equal(node.getchaintips()[0]["height"], 214)
# We need a few more blocks so we can actually get above an realistic
```

🤔 ismaelsadeeq reviewed a pull request: "wallet: Write best block to disk before backup"
(https://github.com/bitcoin/bitcoin/pull/30678#pullrequestreview-2318004044)
code review ACK 38648f4940eb13ebc858081c63e587156428107a

nice adding 38648f4940eb13ebc858081c63e587156428107a 👍🏾

💬 hodlinator commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1768447740)
While posting this PR I was aware of your `tuple_cat` suggestion. I am not against it on principle but TBH I haven't had a clear reason to struggle through that corner of C++ and don't see your suggestion compelling enough to work through it. Your suggestion in that comment is close to compiling but doesn't handle sending in one arg less for the failure cases. A motivation behind this PR is to prove parity in a clear way, and I'm worried it might obfuscate it. Happy to learn more from a diff.

...

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2363533007)
The full tail of the log is below. It shows that 202 fuzz targets have been selected, but only 201 completed.

Not sure which one is missing, but it seems odd that one target would take more than 1.5 hours, when previously everything passed in less than .5 hours.

<details><summary>full log tail</summary>

```
2024-09-20T09:37:21.3001180Z ALL | ✓ Passed | 2778 s (accumulated)
2024-09-20T09:37:21.3001780Z [0mRuntime: 447 s
2024-09-2
...

💬 maflcko commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#discussion_r1768457266)
Thanks, it is fine. I may take a look next week to recover the diff from when I wrote this earlier this month, but this minor style nit certainly isn't important at all.

🤔 hebasto reviewed a pull request: "build: drop obj/ subdirectory for generated build.h"
(https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2318027774)
My Guix build:
```
aarch64
f65f7839cb8ce4b194ab55a698b9840dd335d790a5499d3303cf79885405bedb guix-build-7025942687fd/output/aarch64-linux-gnu/SHA256SUMS.part
7953c080586b3500c73edd93731b8b9bcfd79ac4f5fe7953d51dc54643899255 guix-build-7025942687fd/output/aarch64-linux-gnu/bitcoin-7025942687fd-aarch64-linux-gnu-debug.tar.gz
c82339ac1813727df996deda765087d430080a62d5fff7e16f46727be703a68c guix-build-7025942687fd/output/aarch64-linux-gnu/bitcoin-7025942687fd-aarch64-linux-gnu.tar.gz
ffe2b02e
...

🤔 hebasto reviewed a pull request: "build: drop obj/ subdirectory for generated build.h"
(https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2318031732)
Post-merge re-ACK 7025942687fd5e91d0a10ce5b2ac673b67a63491.

💬 maflcko commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1768466480)
Sure, done!

💬 maflcko commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2363634967)
Thanks! However, I think there was some kind of issue with the fuzz inputs, so that the macOS 14 CI ran into timeouts.

I've removed them temporarily again in https://github.com/bitcoin-core/qa-assets/commit/84cea7068728bc2c765b9da680eedcb85f9f3c1b . This should allow for some time to investigate, while unbreaking the CI.

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2363637034)
Sorry, wrong alarm. The timeout may be real. It may be `p2p_headers_presync` from https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2363634967