💬 fanquake commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3327427018)
The initial issue was for a Guix built binary though: https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3312489843 ?

💬 hebasto commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3327444588)
> The initial issue was for a Guix built binary though: [#33434 (comment)](https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3312489843) ?

That comment doesn’t make sense: "install libxcb-cursor0" cannot result in "removed dark mode". The user likely mixed up the binaries they ran.

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3327532508)
Updated 2ac9d60c54a777978101c369f5895a933208a44c -> 21b0503c2f19f5e4662cea1ceecb425b8460967b ([kernelApi_66](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_66) -> [kernelApi_67](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_67), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_66..kernelApi_67))

* Expanded test coverage a bit more, no changes to the API.

💬 bitschmidty commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3327584051)
> This is the 3rd PR since #32406's merge discussing what the config options' docs should say.

Thank you to all who have reviewed and provided thoughtful feedback on this PR.

> I believe this PR tries to align the documentation with what users can expect. Usage of these options can cause the node to reject transactions that are likely to be mined, so the docs should discourage their use (to me, that means deprecated). However, it seems that removal in the near future is (1) unlikely and (
...

🤔 hebasto reviewed a pull request: "depends: static libxcb-cursor"
(https://github.com/bitcoin/bitcoin/pull/33434#pullrequestreview-3262163206)
Concept ACK. This is consistent with our [docs](https://github.com/bitcoin/bitcoin/blob/master/depends/packages.md#secondary-dependencies).

💬 maflcko commented on pull request "ci: run s390x job":
(https://github.com/bitcoin/bitcoin/pull/33436#issuecomment-3327728312)
> For the record a big-endian CI machine would have helped with #31144

It should be trivial to run the CI task locally, on demand. For development/debugging that should even be easier than to rely on a remove short-lived server. It should also be easy to temporarily add the config to the CI to trigger it on demand, if needed, similar to *BSD stuff, like https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2364674969.

Again, I am not against adding this. My only concern is that this wo
...

💬 maflcko commented on issue "ci: add (atleast one) *BSD job to the CI":
(https://github.com/bitcoin/bitcoin/issues/33438#issuecomment-3327754865)
This should be trivial to add, but requires a third-party action to be enabled. E.g.

* https://github.com/cross-platform-actions/action?tab=readme-ov-file#supported-platforms
* https://github.com/vmactions#hi-there-
* ...

So the main question here is how comfortable are we in trusting those actions to be enabled and run in this repo.

💬 hebasto commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3327769226)
@laanwj

You might want to take a look at this PR.

💬 theDavidCoen commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3327813332)
> > This is the 3rd PR since #32406's merge discussing what the config options' docs should say.
>
> Thank you to all who have reviewed and provided thoughtful feedback on this PR.
>
> > I believe this PR tries to align the documentation with what users can expect. Usage of these options can cause the node to reject transactions that are likely to be mined, so the docs should discourage their use (to me, that means deprecated). However, it seems that removal in the near future is (1) unlik
...

💬 janb84 commented on pull request "guix: documented shasum gathering command":
(https://github.com/bitcoin/bitcoin/pull/33472#issuecomment-3327869054)
Addressed all the feedback (including the linter's spelling sugestion )

💬 ryanofsky commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3327870842)
> Tangibly, that would mean closing this PR

FWIW, I like the current PR more than these other ideas. I see these options as basically the same as other options. Not particularly dangerous, not particularly interesting, likely to be kept if used and there are contributors willing to maintain them, and likely to be removed if not used or they impose significant costs. I think having a warning which draws special attention to these options just amps up partisans on both sides and does not help t
...

💬 trevarj commented on pull request "guix: documented shasum gathering command":
(https://github.com/bitcoin/bitcoin/pull/33472#issuecomment-3327942653)
ACK [d29ab99](https://github.com/bitcoin/bitcoin/commit/d29ab9946f3c1916032f41e7a365dfcb26af2c46)

Thanks for this

💬 trevarj commented on pull request "contrib: Add zsh completion scripts":
(https://github.com/bitcoin/bitcoin/pull/33402#issuecomment-3327956176)
@fanquake this seems useful and I think the bot made a mistake. Could it be reopened?

🤔 maflcko reviewed a pull request: "contrib: Add zsh completion scripts"
(https://github.com/bitcoin/bitcoin/pull/33402#pullrequestreview-3262619549)
You'd have to update the line in the docs? https://github.com/bitcoin/bitcoin/blob/3b5e6121f58f0ca4663deabe793a26be38b0f535/contrib/README.md?plain=1#L45

💬 maflcko commented on pull request "contrib: Add zsh completion scripts":
(https://github.com/bitcoin/bitcoin/pull/33402#discussion_r2375595433)
nit: for new files, you can drop the year range, or use `2025-present`, to avoid having to touch it again in the future.

👍 TheCharlatan approved a pull request: "rpc: fix getblock(header) returns target for tip"
(https://github.com/bitcoin/bitcoin/pull/33446#pullrequestreview-3262639879)
ACK bf7996cbc3becf329d8b1cd2f1007fec9b3a3188

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-3328166580)
`5d7b70161d...e4e13ce4c0`: rebase due to conflicts

Part of this PR was merged via https://github.com/bitcoin/bitcoin/pull/33388, so drop that bit from here. Thanks!

🤔 ismaelsadeeq reviewed a pull request: "rpc: fix getblock(header) returns target for tip"
(https://github.com/bitcoin/bitcoin/pull/33446#pullrequestreview-3262538477)
Code review ACK bf7996cbc3becf329d8b1cd2f1007fec9b3a3188

Comments are on tests and doc

💬 ismaelsadeeq commented on pull request "rpc: fix getblock(header) returns target for tip":
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375540370)
In "test: add block 2016 to mock mainnet" 4c3c1f42cf705e039751395799240da33ca969bd

nit: commit message typo
s/retarger_period/retarget_period/g

💬 ismaelsadeeq commented on pull request "rpc: fix getblock(header) returns target for tip":
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375610799)
In "test: add block 2016 to mock mainnet" 4c3c1f42cf705e039751395799240da33ca969bd

This is a bit confusing @Sjors you updated the name to `halving_period` but still use the regtest retaget period constant.

The constant should be updated `REGTEST_SUBSIDY_HALVING_INTERVAL` for regtest and also define the interval for mainet too, it can be reused in other places.