💬 Sjors commented on issue "Millisecond log timestamp (option)":
(https://github.com/bitcoin/bitcoin/issues/27313#issuecomment-1481317723)
Might be worth promoting this from `OptionsCategory::DEBUG_TEST` to `OptionsCategory::OPTIONS`, since it's useful beyond debugging the software.

⚠️ fanquake opened an issue: "tidy: enable `cppcoreguidelines-pro-type-member-init`"
(https://github.com/bitcoin/bitcoin/issues/27315)
https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-type-member-init.html
> The check flags user-defined constructor definitions that do not initialize all fields that would be left in an undefined state by default construction, e.g. builtins, pointers and record types without user-provided default constructors containing at least one such type. If these fields aren’t initialized, the constructor will leave some of the memory in an undefined state.

Have been multiple calls t
...

💬 mzumsande commented on pull request "p2p: Improve diversification of new connections":
(https://github.com/bitcoin/bitcoin/pull/27264#issuecomment-1481322628)
> Moreover I guess doing any kind of "netgroup"-based diversification for these peers doesn't make sense, since for those addresses, our group assignments are arbitrary and there's no relationship between the address and our route? (If that understanding is correct then that is a separate issue from the improvement here, so could be picked up in another PR.)

I agree, and I think there is another problem that is exacerbated by this PR:

The grouping is not just arbitrary for these networks,
...

💬 fanquake commented on pull request "[DEMO] Integrate `bitcoin-tidy` clang-tidy plugin - warn on uninitialized members of aggregate types":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1481323268)
Going to close this for now. Opened #27315 in regards to adding `cppcoreguidelines-pro-type-member-init` to clang-tidy.

✅ fanquake closed a pull request: "[DEMO] Integrate `bitcoin-tidy` clang-tidy plugin - warn on uninitialized members of aggregate types"
(https://github.com/bitcoin/bitcoin/pull/26296)

💬 hebasto commented on pull request "guix: import/sync python-lief (0.12.3) package definition from upstream":
(https://github.com/bitcoin/bitcoin/pull/27296#issuecomment-1481341580)
Guix builds:
```
2780a866c6a930a413cab8de3840d01c3166712d12d2f17693f30cb162eb4b92 guix-build-24f26e08cc0d/output/aarch64-linux-gnu/SHA256SUMS.part
13ccce6016de14a892b2cfac52d0a412e2c348862871a8b38498ceaf352cf48a guix-build-24f26e08cc0d/output/aarch64-linux-gnu/bitcoin-24f26e08cc0d-aarch64-linux-gnu-debug.tar.gz
3e7c7ea6fde2cdf50f4a8c66baf5ae86295f4b426fef62f32d91839a770149ce guix-build-24f26e08cc0d/output/aarch64-linux-gnu/bitcoin-24f26e08cc0d-aarch64-linux-gnu.tar.gz
9a25be8a68331fb4fa9
...

💬 hebasto commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1481344639)
> Looks like we can't drop the patch, as we'd still end up needing gperf

Maybe include `gperf` in our build tools (like `bison`)?

> In any case, even if fontconfig builds, Qt doesn't work (out of the box) with clang-16, so this seems like a waste of time.

Fixed in #27314.

💬 beirut-boop commented on pull request "wallet: unify “allow/block other inputs“ concept":
(https://github.com/bitcoin/bitcoin/pull/25118#issuecomment-1481344902)
@furszy I understand, concur, and have removed the link from my comment.

> Time is a scarce resource for everyone.

I agree. That's why I thought a quick assessment on the observed issue with the coin control classes before spending time setting up a Bitcoin testnet wallet with the intention of taking up more of your time to look into it was a good approach. The project is different but the coin control classes are identical, so I assumed I was helping everyone.

Apologies. I will not cro
...

💬 fanquake commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1481351232)
> Maybe include gperf in our build tools (like bison)?

I'd rather not add even more gui-only requirements, and dependencies to depends or our release env.

Going to close this, as I don't think I'm going to follow up.

✅ fanquake closed a pull request: "depends: fontconfig 2.14.2"
(https://github.com/bitcoin/bitcoin/pull/27301)

💬 fanquake commented on pull request "compat: move (win) S_* defines into bdb":
(https://github.com/bitcoin/bitcoin/pull/26832#issuecomment-1481355808)
> I feel like I'm missing some context here. Why move these out of compat?

My plan was to split compat up further, so we didn't have the singular compat.h, with everything thrown in.

🚀 fanquake merged a pull request: "test: Replace threading with concurrent.futures"
(https://github.com/bitcoin/bitcoin/pull/27287)

💬 ryanofsky commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1481372074)
CI's hate this one weird PR.

Updated c661be4e7fd9abe5bbec0a9b123dc66d04ffc1bf -> 780c696fc310a6df8464b40983b23f8b0a3074f0 ([`pr/ignoredconf.3`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.3) -> [`pr/ignoredconf.4`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ignoredconf.3..pr/ignoredconf.4)) to fix CI errors

---

```
tuple[] TypeError: 'type' object is not subscriptable`
```
https://cirrus-ci
...

👋 hebasto's pull request is ready for review: "clang-tidy: Add more `performance-*` checks and related fixes"
(https://github.com/bitcoin/bitcoin/pull/26642)

💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1481382733)
Rebased on top of the merged #27311.

All @martinus's comments have been addressed.

💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1146360114)
[Updated](https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1481382733).

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1481390885)
> > It would be better to have two separate commits - one with the non-functional optimization and one with the fix.

> Agreed, that could probably be improved.

I can do this.


> > Sure, lets first see what happens with this PR. Maybe @pablomartin4btc would decide to check the return value of evhttp_request_get_uri() in this PR.

It was under my radar and I thought to do it in a follow up, but I could do it here if it's more practical (need to update title and description of this PR)
...

📝 hebasto converted_to_draft a pull request: "clang-tidy: Add more `performance-*` checks and related fixes"
(https://github.com/bitcoin/bitcoin/pull/26642)
null

💬 fanquake commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1481400657)
Seeing lots of https://cirrus-ci.com/task/6739422046060544?logs=ci#L339:
```bash
/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/rpc/server.cpp:88:9: error: 'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation,-warnings-as-errors]
vCommands.push_back(make_pair(entry.second.front()->category + entry.first, entry.second.front()));
^
/tmp/cirrus-ci-build/ci/scratch/bui
...

💬 fanquake commented on pull request "p2p: remove adjusted time":
(https://github.com/bitcoin/bitcoin/pull/25908#issuecomment-1481401163)
Rebased on master. @ajtowns putting aside the current comments, any thoughts on including changes like this in bitcoin-inquisition?