💬 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)

💬 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_r1157719967)
Could set only the first enum value for smaller diffs/less churn in future changes.

👍 TheCharlatan approved a pull request: "build: remove ancient unused define"
(https://github.com/bitcoin/bitcoin/pull/27420)
ACK 9fbc5fcd28eeefd3ad1932a2d94e5558deeb16d7

💬 jamesob commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1496579012)
Big concept ACK. Will test next week (post-honeymoon).

💬 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_r1157742775)
> In general, I think I'd start with the date column first, and then the warnings one.

I see that Jonas Schnelli used a date-first order as well in https://github.com/bitcoin/bitcoin/issues/3314#issuecomment-81567665.

💬 TheCharlatan commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1496581866)
Updated 93c56f4f203f3bf3a616863b4ec6443aa36b2f9d -> c02e451882bb1d220b944af3b444d4b529ac351a ([kernelChainName_0](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_0) -> [splitSystemArgs_1](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemArgs_0..splitSystemArgs_1)):
* Addressed @MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1157536088) for cleaning up a needl
...

💬 TheCharlatan commented on pull request "compat: move (win) S_* defines into bdb":
(https://github.com/bitcoin/bitcoin/pull/26832#issuecomment-1496583438)
re-ACK 54e406118926c2a526455a60f67961520dfb65da

💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1157778909)
Not sure what you mean, I think this just intends to say `verify.py` ?