💬 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.
(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
(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.
(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.
(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
...
(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?
(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.
(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!).
(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)
(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
(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
(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.
(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?
(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
(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.
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513714579)
Bringing out of draft after finishing the manual seeds updates and taking review feedback.
👋 jonatack's pull request is ready for review: "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x"
(https://github.com/bitcoin/bitcoin/pull/27488)
(https://github.com/bitcoin/bitcoin/pull/27488)
📝 Muhammedhamid23 opened a pull request: "2023"
(https://github.com/bitcoin/bitcoin/pull/27489)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/27489)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ pinheadmz closed a pull request: "2023"
(https://github.com/bitcoin/bitcoin/pull/27489)
(https://github.com/bitcoin/bitcoin/pull/27489)
💬 jonatack commented on pull request "p2p: update hardcoded mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513731469)
> cc @Emzy
Yes. We checked in earlier today. Probably late evening now, pinged him over IRC to have a look.
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513731469)
> cc @Emzy
Yes. We checked in earlier today. Probably late evening now, pinged him over IRC to have a look.
📝 fanquake locked a pull request: "2023"
(https://github.com/bitcoin/bitcoin/pull/27489)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/27489)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...