💬 l0rinc commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2315091137)
The main leveledb repo doesn't even compile for the past 8 months, not sure why we would wait for them. But as I mentioned, I don't mind doing it after the PR, just seems cleaner if we didn't need to change this section again.

💬 Sjors commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2315092723)
And one that I spun up in June and his been online since:

```
915M testnet4/chainstate/
20G testnet4/blocks/
21G total
```

💬 l0rinc commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2315093684)
I know, that wasn't my question. But I'll figure it out next, feel free to resolve it.

💬 saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#issuecomment-3244074756)
Hi @furszy kindly acknowledge this review its been waiting for very long time.

💬 Sjors commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2315145587)
Intermittently online node (though that shouldn't make a difference on this signet):

```
2,8G signet/chainstate/
15G signet/blocks/
18G total
```

💬 maflcko commented on pull request "build: Set AUTHOR_WARNING on warnings":
(https://github.com/bitcoin/bitcoin/pull/33144#issuecomment-3244099875)
Sounds good. Happy to put this in draft for now to see if your removal of `configure_warnings` makes it in, but I haven't looked at 2. and 3., if they make sense.

💬 maflcko commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-3244110633)
What was the speedup? I can only see your last comment, which says:

> the PR is still in draft mode until I remeasure its effect on IBD

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2315254202)
https://github.com/bitcoin/bitcoin/blob/7cc9a087069bfcdb79a08ce77eb3a60adf9483af/src/index/base.h#L116

👍 hodlinator approved a pull request: "clang-format: make formatting deterministic for different formatter versions"
(https://github.com/bitcoin/bitcoin/pull/32813#pullrequestreview-3175194035)
re-ACK 13f36c020f0329b5e975282b45292fdf2a495e31

`struct` brace style was made to explicitly differ from `class` in developer-notes.md after some back & forth (https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2303151367).

💬 hodlinator commented on pull request "clang-format: make formatting deterministic for different formatter versions":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2315150488)
nit:
https://clang.llvm.org/docs/ClangFormatStyleOptions.html#reflowcomments

We might want to set `IndentOnly`, so we don't unnecessarily touch lines. But probably better to follow up on this in another PR, similar to initializer lists (https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2294742258).

💬 hodlinator commented on pull request "clang-format: make formatting deterministic for different formatter versions":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2315284495)
Noting down further arguments for the current version of the PR:
* The fact that *developer-notes.md* already explicitly mentions *functions & methods* means class & struct were probably intentionally included & omitted respectively.
* `AfterStruct` and `AfterClass` were both introduced as part of LLVM 3.8.0 in 2016 (See https://releases.llvm.org/3.8.0/tools/clang/docs/ClangFormatStyleOptions.html vs https://releases.llvm.org/3.7.0/tools/clang/docs/ClangFormatStyleOptions.html), yet only the l
...

💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3244309188)
re-ACK 3c5da69a232ba1cfb935012aa53e57002efe0d77 🏗

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 3c5da69a232ba1cfb935
...

💬 maflcko commented on pull request "clang-format: make formatting deterministic for different formatter versions":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2315342371)
> similar to initializer lists ([#32740 (comment)](https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2294742258)).

When formatting initializer lists, one can manually put the `:` on the next line. This should give a stable clang-format output. (With the current line limit, clang-format won't split or merge lines by itself)

💬 maflcko commented on pull request "clang-format: make formatting deterministic for different formatter versions":
(https://github.com/bitcoin/bitcoin/pull/32813#issuecomment-3244360723)
re-ACK 13f36c020f0329b5e975282b45292fdf2a495e31 🖼

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 13f36c020f0329b5e975
...

🤔 optout21 reviewed a pull request: "doc: unify `datacarriersize` warning with release notes"
(https://github.com/bitcoin/bitcoin/pull/33224#pullrequestreview-3175541576)
ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442

Minute details like this in documentation improve the communication of the project.
The change is doc-only, minimal, well isolated.

💬 dergoegge commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3244443381)
7a6b3b4ebfc04f9fa4520e3caaeeae9dfe0d8c5f still mentions `assert` in the commit message but it's now using `Assume`.

Otherwise this looks good to me.

💬 optout21 commented on pull request "mini miner: enable `Linearize` return package feerates":
(https://github.com/bitcoin/bitcoin/pull/33216#discussion_r2315422039)
nit: Add a comment on the struct?

👍 vasild approved a pull request: "net: Prevent node from binding to the same `CService`"
(https://github.com/bitcoin/bitcoin/pull/33231#pullrequestreview-3175416151)
ACK 28fb7f74c4ae66dcb0f918774d644ac3c393f25b

Some non-blocker comments below, would be happy to re-review if addressed.

💬 vasild commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2315309450)
```suggestion
* @param[in] connOptions Connection options containing the binding vectors to check
```

💬 vasild commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2315349643)
It wasn't clear to me what the return value is before reading the comment, so this is useful. Sometimes comments are more on the side of redundant/noisy/useless, but the line between that and useful comments is blurred and personally I prefer to err on the side of redundancy.

If there are no comments whatsoever, this sets a standard. Then if the function is extended with not-so-obvious parameters or return value changed, the developer doing it will likely not add a comment for the new paramet
...