🤔 promag reviewed a pull request: "qt6: Fix linking when configured with `-DENABLE_WALLET=OFF`"
(https://github.com/bitcoin-core/gui/pull/837#pullrequestreview-2346444309)
I'll try a different approach.

👍 jarolrod approved a pull request: "depends: Print ready-to-use `--toolchain` option for CMake invocation"
(https://github.com/bitcoin/bitcoin/pull/31008#pullrequestreview-2346464174)
ACK 605926da0ab4ac7ae4e9aed55a059f37c31c15b5

👍 jarolrod approved a pull request: "qt6: Handle deprecated `QLocale::nativeCountryName`"
(https://github.com/bitcoin-core/gui/pull/838#pullrequestreview-2346481059)
ACK 9123a286e9743196307d38dd66ae0cfaf3805029

Built this on macOS with qt5.15 and Qt6.7, as well as on Linux Ubuntu with Qt6.4 and Qt6.7, and tested GUI functions related to translations.

💬 hebasto commented on pull request "qt6, test: Handle deprecated code":
(https://github.com/bitcoin-core/gui/pull/839#discussion_r1786715136)
Thanks! Reworked per your feedback.

💬 hebasto commented on issue "Unable to cross compile on linux for macos (28.x branch)":
(https://github.com/bitcoin/bitcoin/issues/31027#issuecomment-2392146216)
@JayBitron

> -- The C compiler identification is Clang 14.0.6

Please ensure the minimum required Clang version, which is [16.0](https://github.com/bitcoin/bitcoin/pull/30263) for v28.0.

Additionally, you may need the [`lld`](https://packages.debian.org/bookworm/lld) package installed (the same version as llvm/clang).

💬 JayBitron commented on issue "Unable to cross compile on linux for macos (28.x branch)":
(https://github.com/bitcoin/bitcoin/issues/31027#issuecomment-2392151361)
@hebasto Altho the stable version of debian provides the clang 14, I have done also the clang 16 and even upper version, they throw different errors

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1786696368)
Done. Added the jump line. I don't feel strong about the capitalization neither but it looks incorrect to introduce it now when all other functions aren't using it.

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1786729786)
> Why not make `SetupUnitTestArgs` available and call it here instead? Delegates the responsibility of documenting the default value to that part of the code.
>
> (Could maybe rename `SetupUnitTestArgs` -> `SetupTestArgs` or `static BasicTestingSetup::RegisterArgs` to make it less unit test-specific and communicate how broadly it is used).

I think we should be careful and explicit about the features we add to other binaries. One of them could end up clashing with another functionality or b
...

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1786698951)
Have reworked this function in a more general manner just so we can add new args in the future with just one line of parsing code.

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1786733103)
done.

🚀 hebasto merged a pull request: "qt6: Handle deprecated `QLocale::nativeCountryName`"
(https://github.com/bitcoin-core/gui/pull/838)

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2392158642)
Thanks for the review @hodlinator. Updated per feedback.

> Good to see these globals implemented for bench. As already mentioned, I've been working on a partially overlapping change in https://github.com/bitcoin/bitcoin/pull/30737.

Cool. It wasn't on my radar. Adding it now.

💬 JayBitron commented on issue "Unable to cross compile on linux for macos (28.x branch)":
(https://github.com/bitcoin/bitcoin/issues/31027#issuecomment-2392168251)
@hebasto lld is installed
```
root@localhost:~/.build/bitcoin-macos# make HOST=x86_64-apple-darwin -C depends -j32
make: Entering directory '/root/.build/bitcoin-macos/depends'
Configuring libevent...
-- The C compiler identification is Clang 20.0.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - failed
-- Check for working C compiler: /usr/bin/clang
-- Check for working C compiler: /usr/bin/clang - broken
CMake Error at /usr/share/cmake-3.30/Modules/CMakeTestCComp
...

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1786755937)
True. I'll leave this for a potential follow-up

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1786756748)
I'll leave this one for follow-up as well.

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1786766139)
We seem to be limited/selective in testing for hidden RPCs in functional tests (i.e. there are more hidden RPCs than `addpeeraddress` and `getrawaddrman`). Maybe that sort of check would/could be a larger scope than just `getorphantxs`?

I'm on the fence about testing that RPCs are hidden. Changing from hidden to another category would be noticed in a PR review, and having a functional test for it means that more code needs to be maintained.

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2392209854)
Thanks for the review @danielabrozzoni and @pablomartin4btc!

> Tested on `mainnet`. Not sure if would be use cases to add other arguments on a follow-up (count, filter by node id)

Yes, those sound interesting for a follow-up or something like `getorphanageinfo`.

👍 hebasto approved a pull request: "ci: set a ctest test timeout of 1200 (20 minutes)"
(https://github.com/bitcoin/bitcoin/pull/31026#pullrequestreview-2346588950)
ACK 93dda4c70254750a5b5c0e7f7d2d89032519b999, I agree with the PR goal and the choice of timeout value.

💬 hebasto commented on pull request "ci: set a ctest test timeout of 1200 (20 minutes)":
(https://github.com/bitcoin/bitcoin/pull/31026#issuecomment-2392236722)
> I tried for quite some time to get this to work using `CTEST_TEST_TIMEOUT` from within _CMakeLists.txt_ but it does not appear to be possible.

According to CMake docs, [`CTEST_TEST_TIMEOUT`](https://cmake.org/cmake/help/latest/variable/CTEST_TEST_TIMEOUT.html)
> Specify the CTest `TimeOut` setting in a `ctest` [dashboard client script](https://cmake.org/cmake/help/latest/manual/ctest.1.html#ctest-script).

which is a use case different from ours.

💬 hodlinator commented on pull request "Windows bitcoind stall debugging [WIP, NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2392267812)
Had [2 scheduled runs](https://github.com/hodlinator/bitcoin/actions) on my [modified master](https://github.com/bitcoin/bitcoin/compare/refs/heads/master...hodlinator:bitcoin:refs/heads/master), 3 hours apart.
- Dump file generation upon bitcoind connection failure is enabled for functional tests, and no longer artificially forced.
- Re-enabled unit tests to keep the setup as close as possible to regular CI.