💬 TheCharlatan commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1513555787)
Updated de2546e672d0a2103228d777b35bfa6aab4881ee -> ec51c252de42e11906f114ac05e476fd88ca2176 ([splitSystemArgs_2](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_2) -> [splitSystemArgs_3](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemArgs_2..splitSystemArgs_3)):
* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/27419#pullrequestreview-1374037220) by adding a config
...

💬 jonatack commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170370860)
Will drop in next push, thanks.

💬 fanquake commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170376722)
I think this can also just be left as-is. "After removing duplicates" certainly fits the tone/amongst the other sentences better. Not really sure why it needs changing.

💬 jonatack commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170387609)
Every other sentence takes the form of a verb followed by an action. I don't mind dropping that, but it seems more in line with the other printed lines.

💬 jonatack commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170390258)
Skip/remove/enforce/require etc.

💬 fanquake commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170392722)
I don't really mind, I just don't see why there's multiple unrelated changes in this PR (manual copyright header bumping, rewriting log output etc), when it could just be a straight-forward seed update.

👍 ryanofsky approved a pull request: "refactor: Extract common/args from util/system"
(https://github.com/bitcoin/bitcoin/pull/27419#pullrequestreview-1390630091)
Code review ACK ec51c252de42e11906f114ac05e476fd88ca2176. Hoping this moveonly change could be reviewed and merged quickly since it conflicts with various PR's I have open

💬 ryanofsky commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170374206)
In commit "move-only: Extract common/args and common/config.cpp from util/system" (ec51c252de42e11906f114ac05e476fd88ca2176)

Any reason these header are only included if WIN32 is not defined? I know the ifndef is preexisting, but it seems like it would make more sense to make these includes unconditional.

💬 ryanofsky commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170386744)
In commit "move-only: Extract common/args and common/config.cpp from util/system" (ec51c252de42e11906f114ac05e476fd88ca2176)

Seems like extra whitespace this line.

Note for reviewers: This function previously wasn't exposed as part of the header, but was moved here as part of splitting up args and config file. It can go away with a followup change making config parsing functions more independent.

💬 fanquake commented on pull request "refactor (tidy): Fixes after enable-debug configure":
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1513602373)
> What are possible drawbacks here?

I don't really understand adding it given [your comment above](https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1494267607):

> I'd avoid it as it will change code paths being parsed now

You suggested avoiding changing the code paths being parsed, and then suggested adding a define that changes the code paths being parsed?

In its current state, this PR is adding a Boost define, because it has a side-effect that basically "tricks" `clang-ti
...

💬 TheCharlatan commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170434860)
I checked it and its history again and can't think of a good reason either. Should I amend here, or go for the follow up?

💬 mzumsande commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1170436504)
done, although I think we should assert `nAddSize < max_blockfile_size;` - see explanation above.

💬 mzumsande commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#issuecomment-1513632824)
Thanks for the reviews! I pushed an update to address the outstanding comments and added the test by @pinheadmz (thanks!).

💬 MarcoFalke commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170443781)
```suggestion
" src/common/args.cpp"\
" src/common/config.cpp"\
```

nit (if you retouch)

💬 Nimano1981 commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1513649390)
@hebasto

💬 Nimano1981 commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1513649561)
#27191

💬 theuni commented on pull request "depends: fix compiling bdb with clang-16 on aarch64":
(https://github.com/bitcoin/bitcoin/pull/27462#issuecomment-1513651367)
Just in case it's ever helpful (or as @MarcoFalke said, in case we need this for c++20), here's a set of depends patches for bdb that should fix the existing configure checks: https://github.com/theuni/bitcoin/commits/bdb-configure-int-checks

The code itself still isn't patched for the missing prototypes, but if the time comes and we need to do that, it should be simple enough.

💬 TheCharlatan commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170497181)
`config.cpp` still produces the <fstream> false positive. Is that acceptable?

💬 fanquake commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513714252)
cc @Emzy

💬 jonatack commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513714579)
Bringing out of draft after finishing the manual seeds updates and taking review feedback.