💬 martinus commented on pull request "tracepoints: Disables `-Wgnu-zero-variadic-macro-arguments` to compile without warnings":
(https://github.com/bitcoin/bitcoin/pull/27401#issuecomment-1496410442)
> Maybe add a check to configure.ac whether ignoring of -Wgnu-zero-variadic-macro-arguments is https://github.com/bitcoin/bitcoin/issues/26916#issuecomment-1400156218 required?

Sounds good, but I have no idea how to do this with `configure.ac`... But it's also possible to do this in code level, I've now added a `__cplusplus < 202002L` so disabling the warning only happens when compiling below c++20

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#issuecomment-1496413288)
Thanks all for the feedback :). [Update Diff](https://github.com/bitcoin/bitcoin/compare/ba7970c793f3e8ddb54a02256e7ae52e88e47f08..488dbeab56b2cd62e381883e5fe347b446ae7d2c):

* Renamed `max_weight` to `max_inputs_weight`.
* Deduced change size from the max allowed weight for Knapsack and SRD (not from BnB as it's specialized to not create change).
* Adapted doxygen comment to current sources.

💬 RandyMcMillan commented on pull request "build: remove ancient unused define":
(https://github.com/bitcoin/bitcoin/pull/27420#issuecomment-1496440854)
utACK 9fbc5fc

💬 RandyMcMillan commented on pull request "doc: Add example of how to mix private and public keys in descriptors":
(https://github.com/bitcoin/bitcoin/pull/27414#issuecomment-1496446116)
Concept ACK

💬 hebasto commented on pull request "ci: Remove second user account":
(https://github.com/bitcoin/bitcoin/pull/27376#issuecomment-1496449588)
Unfortunately, I've noticed a regression introduced in this PR.

On Ubuntu 22.04, running `FILE_ENV="./ci/test/00_setup_env_native_tidy.sh" ./ci/test_run_all.sh` locally fails with
```
...
IWYU edited 30 files on your behalf.

warning: Not a git repository. Use --no-index to compare two paths outside a working tree
usage: git diff --no-index [<options>] <path> <path>
...
```

👍 hebasto approved a pull request: "compat: move (win) S_* defines into bdb"
(https://github.com/bitcoin/bitcoin/pull/26832)
ACK 54e406118926c2a526455a60f67961520dfb65da, I have reviewed the code and it looks OK.

nit: Two commits _"refactor: don't avoid sys/types.h on when building for Windows"_ and _"compat: move (win) S\_* defines into bdb"_ were squashed but the commit message of the latter was lost.

👍 Ayush170-Future approved a pull request: "build: remove ancient unused define"
(https://github.com/bitcoin/bitcoin/pull/27420)
ACK

💬 MarcoFalke commented on pull request "More verbose warning for multiple network argument error.":
(https://github.com/bitcoin/bitcoin/pull/26028#issuecomment-1496533357)
Closing for now due to lack of activity. Let us know when the should be reopened.

✅ MarcoFalke closed a pull request: "More verbose warning for multiple network argument error."
(https://github.com/bitcoin/bitcoin/pull/26028)

💬 brunoerg commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1496568490)
> Would be nice to log this for inbound connections too, I guess the receive version message: is most suitable for that?

Yes, it would be great to have this for both ones, just updated it!

💬 TheCharlatan commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1157735880)
No I cannot, because it doesn't make sense. Will revert.

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157712788)
Missing brackets here (we require brackets with conditionals over one line due to CVE-2014-1266, the Apple "goto fail" vulnerability, see also https://dwheeler.com/essays/apple-goto-fail).

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157714528)
nit, I prefer to read left-to-right rather than up-to-down, when these can all be on one line

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157715369)
Could set only the first enum value for smaller diffs/less churn in future changes.

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157718871)
(Idem brackets here and lines 118-119)

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157717210)
(Idem brackets.)

nit, looks like this warning is the same one as above at https://github.com/bitcoin-core/gui/pull/723/files#diff-74dd156c97c401ff337c0ed5399192d86ea2120f7baed1368d2a407a74e347cfR74. Maybe define it in one place if it makes sense to do so.

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157721419)
Why not just on one line?

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157716418)
(As above) same line or brackets here.

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157721554)
Why not just on one line?

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157722990)
(Same line or brackets)