💬 hernanmarino commented on pull request "build, qt: Fix handling of `CXX=clang++` when building `qt` package":
(https://github.com/bitcoin/bitcoin/pull/27314#issuecomment-1485282916)
I can confirm with clang-15.
```
Ubuntu clang version 15.0.7
Target: x86_64-pc-linux-gnu
Thread model: posix
```

💬 hebasto commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#discussion_r1149392363)
Is `-Wno-int-conversion` really required to compile with clang-16?

💬 hebasto commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1485289625)
> depends: fontconfig 2.14.2

Update the title?

💬 1440000bytes commented on pull request "rpc, wallet: add ability to retrieve all address book entries":
(https://github.com/bitcoin/bitcoin/pull/26174#issuecomment-1485309914)
Concept ACK

💬 hebasto commented on pull request "Warn when sending to already-used Bitcoin addresses":
(https://github.com/bitcoin-core/gui/pull/562#issuecomment-1485310320)
Closing this due to lack of activity. Feel free to reopen.

✅ hebasto closed a pull request: "Warn when sending to already-used Bitcoin addresses"
(https://github.com/bitcoin-core/gui/pull/562)

💬 hebasto commented on pull request "Make error message layout consistent":
(https://github.com/bitcoin-core/gui/pull/560#issuecomment-1485313776)
Closing this due to lack of activity. Feel free to reopen.

✅ hebasto closed a pull request: "Make error message layout consistent"
(https://github.com/bitcoin-core/gui/pull/560)

💬 hebasto commented on pull request "Network graph improvements":
(https://github.com/bitcoin-core/gui/pull/559#issuecomment-1485316876)
Closing this due to lack of activity. Feel free to reopen.

💬 hebasto commented on pull request "Network graph improvements":
(https://github.com/bitcoin-core/gui/pull/559#issuecomment-1485316876)
Closing this due to lack of activity. Feel free to reopen.

✅ 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
...