✅ hebasto closed a pull request: "Network graph improvements"
(https://github.com/bitcoin-core/gui/pull/559)

💬 hebasto commented on pull request "network graph - show/hide panels based on window width/height":
(https://github.com/bitcoin-core/gui/pull/540#issuecomment-1485322497)
Closing this due to lack of activity. Feel free to reopen.

✅ hebasto closed a pull request: "network graph - show/hide panels based on window width/height"
(https://github.com/bitcoin-core/gui/pull/540)

💬 1440000bytes commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1485326965)
> Show an error on startup if a bitcoin datadir that is being used contains a bitcoin.conf file that is ignored

1. Why is it ignored?
2. Any downsides for using bitcoin.conf from datadir if conf argument isn't used?

💬 furszy commented on pull request "wallet: finish addressbook encapsulation":
(https://github.com/bitcoin/bitcoin/pull/26836#discussion_r1149415708)
I prefer the current code, it's more readable.
The curly braces initializer is primarily useful to prevent narrowing (which is the implicit conversion of arithmetic values). Here, there is no loss of accuracy, we are simply initializing a boolean from a logical statement.

💬 ryanofsky commented on pull request "system: cache config file path before potentially updating datadir":
(https://github.com/bitcoin/bitcoin/pull/27303#issuecomment-1485330420)
Code review ACK 8d6114aeaa7cfc747eae068116da04af86ed8e92. I think this is the probably the simplest possible bugfix for the incorrect log print, but it also adds unnecessary code complexity and fragile code without meaningful test coverage, so I do not think it is the best approach.

Here are the specific problems with 8d6114aeaa7cfc747eae068116da04af86ed8e92:

- The new python test doesn't actually check for the bug being fixed, and passes on existing code. The bug can't be tested unless bi
...

💬 josibake commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#issuecomment-1485343142)
Concept ACK

💬 furszy commented on pull request "wallet: finish addressbook encapsulation":
(https://github.com/bitcoin/bitcoin/pull/26836#discussion_r1149436821)
Thanks but same as above, I prefer current code as it's more readable and there is no possible narrowing issue here.

💬 fanquake commented on pull request "depends: make fontconfig build under clang-16":
(https://github.com/bitcoin/bitcoin/pull/27301#discussion_r1149437866)
Looks like we can just do `implicit-function-declaration` for now. We'll leave all the other warnings-that-may-become-errors for future change.

💬 ryanofsky commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1485364863)
> > Show an error on startup if a bitcoin datadir that is being used contains a bitcoin.conf file that is ignored
>
> 1. Why is it ignored?

It's debatable whether it should be ignored, but the reason it is ignored is that one configuration file has already been parsed and trying to merge in a second configuration file in another datadir would be new and potentially confusing behavior. Merging in another configuration file could lead to unexpected conflicts between settings in the two f
...

💬 hernanmarino commented on pull request "Wallet : Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#discussion_r1149455794)
Yes, will do.

💬 josibake commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1149457810)
IIRC, don't we get all sorts of compiler goodness if we use `switch` with an enum, like compiler warnings for unhandled cases? Would be nice, in the event we ever add a new case to the enum (although seems unlikely for this specific use-case).

I always prefer using `switch` with enums, but feel free to ignore if you don't feel like retouching/don't share the same opinion.

💬 fanquake commented on pull request "ci: Use TSan new runtime (llvm-16, take 3)":
(https://github.com/bitcoin/bitcoin/pull/27298#issuecomment-1485397265)
Looks like the explicit `libclang-rt-dev` package installation was lost, so configure is failing.

👍 hebasto approved a pull request: "depends: make fontconfig build under clang-16"
(https://github.com/bitcoin/bitcoin/pull/27301)
ACK 9cbc1c279247800d79c5f6f95c0c2d8f387aac0a

💬 josibake commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1149481491)
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wswitch-enum , super nice and still warns for unhandled cases even when you have a default.

✅ vstoyanov closed a pull request: "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)"
(https://github.com/bitcoin/bitcoin/pull/27333)

💬 fanquake commented on pull request "fuzz: Remove legacy int parse fuzz tests":
(https://github.com/bitcoin/bitcoin/pull/27344#issuecomment-1485478244)
Concept ACK

✅ fanquake closed an issue: "depends does not compile with clang-16 (fontconfig)"
(https://github.com/bitcoin/bitcoin/issues/27299)

🚀 fanquake merged a pull request: "depends: make fontconfig build under clang-16"
(https://github.com/bitcoin/bitcoin/pull/27301)

💬 vstoyanov commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)":
(https://github.com/bitcoin/bitcoin/pull/27333#issuecomment-1485513160)
Apologies, could we reopen this one? I tried rebasing to retrigger Cirrus CI