💬 stickies-v commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1910229692)
> Are there places where this would be re-used?

I can't quite find one - it's also a bit difficult to search for given how much `<<` is used for streams too. I agree there's no need to generalize too early, but given how similar the functionality is I still have a preference for the more generic `CheckedLeftShift()` and/or `SaturatingLeftShift()` as suggested by @ryanofsky. But, I'm okay with either approach.

> I think I would drop the MiBToBytes function and just write the constants as by
...

🚀 fanquake merged a pull request: "depends: Fix spacing issue"
(https://github.com/bitcoin/bitcoin/pull/31627)

👍 BrandonOdiwuor approved a pull request: "init,log: Unify block index log line"
(https://github.com/bitcoin/bitcoin/pull/31616#pullrequestreview-2542199335)
Code Review ACK e04be3731f4921cd51d25b1d6210eace7600fea4

🚀 fanquake merged a pull request: "tracing: Rename the `MIN` macro to `_TRACEPOINT_TEST_MIN` in log_raw_p2p_msgs"
(https://github.com/bitcoin/bitcoin/pull/31623)

🚀 fanquake merged a pull request: "init,log: Unify block index log line"
(https://github.com/bitcoin/bitcoin/pull/31616)

👍 l0rinc approved a pull request: "net: Disconnect message follow-ups to #28521"
(https://github.com/bitcoin/bitcoin/pull/31633#pullrequestreview-2542183397)
utACK 286de9749612565fd8d42f08f66c8426faf8a1f7

💬 l0rinc commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910228378)
I found the `%d%s` format weird at first, but I see it is an existing format, e.g. https://github.com/bitcoin/bitcoin/blob/master/src/net.cpp#L705

💬 l0rinc commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910230992)
super-nit: above we're using a simple `,` before the `DisconnectMsg`, not a `;`.

💬 l0rinc commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910231692)
nit: many other similar logs here were capitalized

💬 l0rinc commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910245234)
unrelated: do we really need this level of detail for a timeout (i.e. ms precision expressed in seconds)?

💬 hebasto commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2582535849)
From my understanding, the issue arises because the `make` provided by Xcode does not set the `.SHELLFLAGS` variable properly. Here's an example of the output:
```
% whereis make
make: /usr/bin/make /Applications/Xcode.app/Contents/Developer/usr/share/man/man1/make.1
% make --version
GNU Make 3.81
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
PARTICULAR
...

💬 hebasto commented on pull request "depends: fix for llvm-ranlib (etc): 'No such file or directory' macOS 15.0":
(https://github.com/bitcoin/bitcoin/pull/30994#issuecomment-2582537242)
> > Not sure about this issue, or making this change. Looks like it needs more investigation. See my comment here: [#30978 (comment)](https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388086723).
>
> Yes, switched to draft until we have a better understanding of the issue.

See https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2582535849.

💬 fanquake commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2582556693)
What is the status of this?

💬 maflcko commented on pull request "test: importdescriptors is not available for non-descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/31609#issuecomment-2582574580)
Huh? Now you are adding a test for a previous release, which does not increase the test coverage, see also https://corecheck.dev/bitcoin/bitcoin/pulls/31609 .


If you want to test a previous release, it would be better to submit the test against the previous release branch. But I am not sure if this is worth it.

Also, I don't understand why the check should be kept after the bdb removal. Why would one want to guard against bdb at runtime when it is impossible to load bdb?

✅ brunoerg closed a pull request: "test: importdescriptors is not available for non-descriptor wallets"
(https://github.com/bitcoin/bitcoin/pull/31609)

💬 brunoerg commented on pull request "test: importdescriptors is not available for non-descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/31609#issuecomment-2582613729)
> Huh? Now you are adding a test for a previous release, which does not increase the test coverage, see also https://corecheck.dev/bitcoin/bitcoin/pulls/31609 .

Nevermind, my bad, I did it quickly and did not notice it. I'm gonna close it because I believe this check could be removed within bdb removal.

💬 fanquake commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1910323797)
Dropped this out as well.

📝 NicolaLS opened a pull request: "doc: merge required dependency tables"
(https://github.com/bitcoin/bitcoin/pull/31634)
I was a bit confused why there are two tables in [dependencies.md](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) please let me know if there a reason for this or if this change makes sense.

_rationale/motivation:_
Initially there was a distinction between the compiler dependencies and other required dependencies (refs #23565) but the distinction was removed (refs #24585) which is why having two distinct tables could lead to confusion now. This commit merges both tables
...

💬 maflcko commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1910358592)
this is optional

💬 maflcko commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2582680383)
lgtm ACK 08c74105fd65b6356fcdb9d01972fd19f160b588, otherwise