💬 b-l-u-e commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#discussion_r2176889079)
thank you for the suggestion..i will make changes

🤔 hebasto reviewed a pull request: "doc: clarify that the "-j N" goes after the "--build build" part"
(https://github.com/bitcoin/bitcoin/pull/32846#pullrequestreview-2974314528)
Concept ACK. It follows CMake [docs](https://cmake.org/cmake/help/latest/manual/cmake.1.html#build-a-project).

Why not apply such a change across all the docs?

See the output of `git grep -i 'Use "-j N"'` for relevant instances.

💬 fanquake commented on pull request "build, docs: Fix Boost-related issues on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/32828#issuecomment-3022809958)
cc @purpleKarrot

💬 stickies-v commented on pull request "doc: clarify that the "-j N" goes after the "--build build" part":
(https://github.com/bitcoin/bitcoin/pull/32846#issuecomment-3022837089)
Concept ACK

> See the output of git grep -i 'Use "-j N"' for relevant instances.

Agreed with this. I also couldn't find any other patterns that should be updated here.

💬 Sjors commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2176931877)
Narrator: it does

💬 fanquake commented on pull request "depends: fix libevent `_WIN32_WINNT` usage":
(https://github.com/bitcoin/bitcoin/pull/32837#issuecomment-3022864921)
> Any idea why the issue manifests only with mingw-w64 13.x?

It's due to this change: https://github.com/mingw-w64/mingw-w64/commit/6cfc1fd2ca2fdbbb8d6570084970bea2ef100d2e. The headers have started defining `NTDDI_VERSION` based on the value of `_WIN32_WINNT` (if the version was < Windows 10). Given that libevent was undefining our `_WIN32_WINNT`, and then redefining it to a value < Windows 10 (`0x0501`), `NTDDI_VERSION` is also defined to that value, which was leading to functions not being
...

💬 purpleKarrot commented on pull request "build, docs: Fix Boost-related issues on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/32828#issuecomment-3022868889)
ACK 5a5ddbd78922236402df378c8588a7b0b3f83a13

I wonder why no other projects run into this issue.

🤔 BrandonOdiwuor reviewed a pull request: "rpc, doc: clarify watch-only wallets balances in RPCHelp"
(https://github.com/bitcoin/bitcoin/pull/32761#pullrequestreview-2974424721)
Code Review ACK cb44680db40c85ba338f2a054511cd68ba6e89ba after reviewing https://github.com/bitcoin/bitcoin/pull/32618

💬 Eunovo commented on pull request "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs":
(https://github.com/bitcoin/bitcoin/pull/32618#issuecomment-3022945375)
Consider cleaning up `wallet_balance.py` too, like this:
```diff
diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py
index 4efc0169eb..ac0374c3fa 100755
--- a/test/functional/wallet_balance.py
+++ b/test/functional/wallet_balance.py
@@ -80,9 +80,9 @@ class WalletTest(BitcoinTestFramework):
assert_equal(self.nodes[0].getbalance("*"), 50)
assert_equal(self.nodes[0].getbalance("*", 1), 50)
assert_equal(self.nodes[0].getbalance(minco
...

💬 Eunovo commented on pull request "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs":
(https://github.com/bitcoin/bitcoin/pull/32618#issuecomment-3023002708)
Here is a diff removing `include_watchonly=True` from other functional tests, except `wallet_migration.py`
```diff
diff --git a/test/functional/wallet_importprunedfunds.py b/test/functional/wallet_importprunedfunds.py
index cb052e4511..d45be4aa18 100755
--- a/test/functional/wallet_importprunedfunds.py
+++ b/test/functional/wallet_importprunedfunds.py
@@ -89,7 +89,7 @@ class ImportPrunedFundsTest(BitcoinTestFramework):
wwatch = self.nodes[1].get_wallet_rpc('wwatch')
wwa
...

📝 vtjl10 opened a pull request: "Rename CMake required flags variable in crc32c"
(https://github.com/bitcoin/bitcoin/pull/32847)


```markdown

This pull request fixes a typo in the variable name used to restore CMake required flags in `src/crc32c/CMakeLists.txt`

- Corrected `REQURED` → `REQUIRED`
- Ensures the correct variable is referenced when resetting CMake flags

```

✅ fanquake closed a pull request: "Rename CMake required flags variable in crc32c"
(https://github.com/bitcoin/bitcoin/pull/32847)

💬 fanquake commented on pull request "Rename CMake required flags variable in crc32c":
(https://github.com/bitcoin/bitcoin/pull/32847#issuecomment-3023047673)
Changes to this subtree should go to https://github.com/bitcoin-core/crc32c-subtree.

💬 bigspider commented on pull request "doc: clarify that the "-j N" goes after the "--build build" part":
(https://github.com/bitcoin/bitcoin/pull/32846#issuecomment-3023135741)
Done, and rebased.

I also noticed that the all the analogous comments were capitalized in the `build-*.md` files, so I capitalized them in `build-unix.md` as well.

💬 hebasto commented on pull request "cmake: Improve robustness and usability":
(https://github.com/bitcoin/bitcoin/pull/31233#discussion_r2177110134)
These custom targets are meaningful and run only within the context of the Guix environment. [Currently](https://github.com/bitcoin/bitcoin/pull/32782#issuecomment-3018586630), no tests are run there.

💬 hebasto commented on pull request "cmake: Improve robustness and usability":
(https://github.com/bitcoin/bitcoin/pull/31233#discussion_r2177114584)
Sure. However, this is no longer relevant following the recent branch update.

👍 hebasto approved a pull request: "doc: clarify that the "-j N" goes after the "--build build" part"
(https://github.com/bitcoin/bitcoin/pull/32846#pullrequestreview-2974701234)
ACK 0e9f409db3b7b08aef75ce39765b018b69cc8e9d.

💬 stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2177179314)
This should be `&&` instead of `||`. And the reason that the functional test didn't fail with this change is an indication that the `.isNull()` check is not really reliable either, because it will happily accept an empty array.

Do we actually need to ensure that at least one such (valid) parameter is provided? If so, we should probably check that `blockindexes_sorted` and `scripts_to_watch` are not both empty. Otherwise, I think just making sure we only execute the `request.params[n].get_arra
...

👋 hebasto's pull request is ready for review: "cmake: Improve robustness and usability"
(https://github.com/bitcoin/bitcoin/pull/31233)

💬 hebasto commented on pull request "cmake: Improve robustness and usability":
(https://github.com/bitcoin/bitcoin/pull/31233#issuecomment-3023332643)
Rebased and undrafted. The PR description has been updated as well.

Friendly ping @maflcko and @purpleKarrot :)