💬 Sjors commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#issuecomment-3557810718)
Changed the approach to using `string_view` as suggested by @maflcko.

💬 maflcko commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2545909334)
nit: unused/double scope?

💬 maflcko commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2545907849)
nit: I wonder how hard it would be to update most of those to use string_view instead of a span?

💬 maflcko commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2545896220)
stale comment?

💬 maflcko commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2545901557)
string_view is already on `const char` and the const here can be removed. See also a bit related: https://github.com/bitcoin/bitcoin/pull/31650

💬 vasild commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2545913179)
Yeah, I was thinking the same. It is just super odd to ignore database errors (in general). I guess the sqlite API is odd here.

💬 Sjors commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2545923676)
Interesting, on macOS the compiler refused `Parse("pk(0279...798)"` for me with a clear warning that the method was deleted. But anyway, string_view is better.

💬 Sjors commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2545925892)
Dropped.

💬 Sjors commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2545932135)
This used to be two tests, I'll drop the extra scope.

💬 Sjors commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#issuecomment-3557870296)
Addressed nits.

I'll look into if we can also use `string_view` in one or more of the calls we make, see https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2545907849

💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#issuecomment-3557938032)
Lost the first commit in that rebase, let's try again...

💬 stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2546009312)
ah thanks! that makes sense.

💬 stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2546010617)
I've changed `addr_count` to `int`. theoretical max of addrman size is 81,920 (1024 new buckets * 64 entries + 256 tried buckets * 64 entries), so should be enough.

💬 TheCharlatan commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3557987606)
Can this be closed now that #33511 is merged?

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3557995189)
`07650a49f9...e9b436a408`: resolve some typos reported by the linter

💬 fanquake commented on pull request "depends: update xcb-util packages to latest versions":
(https://github.com/bitcoin/bitcoin/pull/33851#issuecomment-3558096658)
Guix Build (x86_64):
```bash
17cd36aa7eaae8cbe242efa364e55699b1568a6d0cb2c2e1ac82781b46a5f174 guix-build-f541b92cf2bb/output/aarch64-linux-gnu/SHA256SUMS.part
69985b56e1c6ff521f4f49ba102cb53275106dfd076be98361068ef113379fa3 guix-build-f541b92cf2bb/output/aarch64-linux-gnu/bitcoin-f541b92cf2bb-aarch64-linux-gnu-debug.tar.gz
4feb585aaf30ecc8b8c1c1fa32e4a8fa9e7f3e67286825e006807f714f15b039 guix-build-f541b92cf2bb/output/aarch64-linux-gnu/bitcoin-f541b92cf2bb-aarch64-linux-gnu.tar.gz
43b3f41
...

💬 waketraindev commented on pull request "Add console commands for clearing output and history":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3558105697)
Tought a few screenshots might support this PR:

<img width="835" height="585" alt="image" src="https://github.com/user-attachments/assets/7305ed09-bb02-40d4-a2db-33f9adade4c1" />

<img width="826" height="581" alt="image" src="https://github.com/user-attachments/assets/505cec38-2a65-472e-ad0a-79758caed25e" />

💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in most CI tasks via `dev-mode`":
(https://github.com/bitcoin/bitcoin/pull/33824#discussion_r2546132811)
@fanquake Happy to look into win-cross-test-qt into a follow up. Here, I mostly want to mostly focus on the kernel stuff and get ci coverage for it.

💬 fanquake commented on pull request "ci: Enable experimental kernel stuff in most CI tasks via `dev-mode`":
(https://github.com/bitcoin/bitcoin/pull/33824#discussion_r2546140735)
No worries, nothing blocking here from me.

💬 waketraindev commented on pull request "Prevent re-execution of sensitive commands from console history":
(https://github.com/bitcoin-core/gui/pull/909#issuecomment-3558206243)
Screenshots to support the PR:

<img width="827" height="585" alt="image" src="https://github.com/user-attachments/assets/52724076-9dd5-4f8e-b8c4-6ce7d59906fa" />

<img width="829" height="584" alt="image" src="https://github.com/user-attachments/assets/2244d976-8a53-4619-ac1d-3ec94dd84098" />