💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1758851197)
thanks! I updated this to `cmake build` instead of `configure script` in [5a85022](https://github.com/bitcoin/bitcoin/pull/30875/commits/5a85022cc6c315e9c756bbbb63a7ed992e7f23d3)

let me know if you'd prefer different wording!

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1758851652)
thanks! I updated this to `cmake build` instead of `configure script` in [5a85022](https://github.com/bitcoin/bitcoin/pull/30875/commits/5a85022cc6c315e9c756bbbb63a7ed992e7f23d3)

let me know if you'd prefer different wording!

💬 laanwj commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2348948414)
> I made a significant change here, making m_position an std::optional<int64_t>, which is std::nullopt when the position is unknown (due to ftell failing, and no successful fseek afterwards).

i first couldn't think of a valid scenario where ftell would fail but we'd want to continue, but i guess this maks sense when reading a FIFO or device, when there isn't really an offset.

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2348968829)
> It would be nice to fix all remaining instances in one go. See also [#30664 (comment)](https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2320892444) and [#30664 (comment)](https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2321610186)

There are still some changes I need to address from these comments

⚠️ fanquake opened an issue: "translations: `28.x` update pulled in random strings?"
(https://github.com/bitcoin/bitcoin/issues/30897)
Transations were updated for 28.x in #30715.

Looks like that update in things that are not translations. i.e:

https://github.com/bitcoin/bitcoin/blob/e43ce250c6fb65cc0c8903296c8ab228539d2204/src/qt/locale/bitcoin_gl_ES.ts#L6

?

💬 fanquake commented on pull request "qt: 28.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/30715#issuecomment-2348975779)
See #30897. This has pulled broken strings.

💬 maflcko commented on pull request "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2348983390)
> The new scripts introduce trailing spaces in the generated header, but these are not essential.

In theory the spaces in the data itself could be dropped completely, because for the compiler (and a human reader) a single `,` without space is sufficient. But I agree it doesn't matter, so probably best to leave as-is for now.

🤔 BrandonOdiwuor reviewed a pull request: "test: Introduce ensure helper"
(https://github.com/bitcoin/bitcoin/pull/30893#pullrequestreview-2303165380)
Concept ACK

💬 hebasto commented on issue "translations: `28.x` update pulled in random strings?":
(https://github.com/bitcoin/bitcoin/issues/30897#issuecomment-2348987750)
> Transations were updated for 28.x in #30715.
>
> Looks like that update pulled in things that are not translations. i.e:
>
> https://github.com/bitcoin/bitcoin/blob/e43ce250c6fb65cc0c8903296c8ab228539d2204/src/qt/locale/bitcoin_gl_ES.ts#L6
>
> ?

This is a poor / malicious translation.

https://app.transifex.com/bitcoin/bitcoin/translate/#gl_ES/qt-translation-028x/508593963:

💬 maflcko commented on issue "translations: `28.x` update pulled in random strings?":
(https://github.com/bitcoin/bitcoin/issues/30897#issuecomment-2348993105)
Aren't LLMs capable of translation? With all the hype around them I wonder if a script can be written to check that each translation pair is a valid translation. With 4o-mini the cost should also be trivial.

💬 fanquake commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2348994520)
Also in `test_deterministic_coverage.sh`: `Run \"./configure --enable-lcov\"`

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1758931875)
Maybe rather `ensure_for(duration=X...`? "after" implies that the condition will only be checked once after time is up IMO...

📝 Sjors opened a pull request: "ci: honor ci bypass prefix in test-each-commit"
(https://github.com/bitcoin/bitcoin/pull/30898)
If the HEAD commit message of a pull request contains specific keywords, it is skipped by Cirrus and/or Github.

Have the "test each commit" job do so as well.

💬 Sjors commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349043197)
Currently testing this in https://github.com/Sjors/bitcoin/actions/runs/10850488710/job/30112077923?pr=62

💬 TheCharlatan commented on pull request "build: add `standard branch-protection` to hardening flags for aarch64-linux":
(https://github.com/bitcoin/bitcoin/pull/30433#issuecomment-2349043252)
Guix build (aarch64):
```
86d20fcaf2331084035fba29305dbcd22665a7a0f910ffbe1667541a338129ed guix-build-001b1cf01045/output/aarch64-linux-gnu/SHA256SUMS.part
06ec1b29ed2ac24733fa24083460298e4c9a2cbba8845164f849aebb50c94e91 guix-build-001b1cf01045/output/aarch64-linux-gnu/bitcoin-001b1cf01045-aarch64-linux-gnu-debug.tar.gz
16b21d25dab9222dc69774e0e788c10bac4f15783c4ccb46c11e0608d132021d guix-build-001b1cf01045/output/aarch64-linux-gnu/bitcoin-001b1cf01045-aarch64-linux-gnu.tar.gz
3cdd10a2eb
...

👍 TheCharlatan approved a pull request: "build: add `standard branch-protection` to hardening flags for aarch64-linux"
(https://github.com/bitcoin/bitcoin/pull/30433#pullrequestreview-2303241380)
ACK 001b1cf010453adbb1316a6fa8911398953afe61

💬 Sjors commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349051964)
Testing this PR here: https://github.com/Sjors/bitcoin/actions/runs/10850635988/job/30112547021?pr=63

💬 TheCharlatan commented on pull request "kernel: Move background load thread to node context":
(https://github.com/bitcoin/bitcoin/pull/30896#discussion_r1758945881)
Yeah, on second thought this is a bit confusing. The default constructed thread should already be not joinable.

💬 maflcko commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349059351)
No strong opinion, but I find it a bit distracting having to think whether to put `ci skip` or not into a commit. Also, it would be distracting to review. Also, it would be distracting to have this permanently in the commit log. Also, it seems like premature optimization and was never used in the last 8 years, according to `git log --grep='ci skip' -1`.

💬 TheCharlatan commented on pull request "kernel: Move background load thread to node context":
(https://github.com/bitcoin/bitcoin/pull/30896#issuecomment-2349062387)
Updated 3a26c839c63ecda2d3b2491d6d84a03c1d8f508f -> bc7900f33db3d01fb93dfee7981c01ea495cd42e ([ChainmanRmThreadLoad_0](https://github.com/TheCharlatan/bitcoin/tree/ChainmanRmThreadLoad_0) -> [ChainmanRmThreadLoad_1](https://github.com/TheCharlatan/bitcoin/tree/ChainmanRmThreadLoad_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/ChainmanRmThreadLoad_0..ChainmanRmThreadLoad_1))

* Addressed @maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/30896#discussion_r1758724516),
...