💬 jonatack commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2353137979)
If I understand correctly, reviewers (or possibly DrahtBot) should signal if `[skip ci]` is being used inappropriately in any case.

If skipping it is considered appropriate for certain changes (e.g. markdown files as mentioned above) to save time and compute resources, then this pull makes sense to me.

If skipping it is discouraged in all cases, then yes, no need for this update -- in which case it may be useful for the bot, if feasible, to signal any case it detects that `[skip ci]` is us
...

💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1761310952)
this doesn't seem to be the case anymore, the above ones recreate the folders

💬 maflcko commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1761324831)
I meant the "bind-mount" part, not the folder creation. (I think creating the folder is best required to be done outside the script, as permission bits need to be set appropriately)

I guess I can just remove the comment?

💬 maflcko commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2353183613)
> If skipping it is considered appropriate for certain changes (e.g. markdown files as mentioned above) to save time and compute resources, then this pull makes sense to me.

I don't think it is appropriate to skip, even for markdown, see the reply above: https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2352219690



> If skipping it is discouraged in all cases, then yes, no need for this update -- in which case it may be useful for the bot, if feasible, to signal any case it det
...

💬 jonatack commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2353192340)
> I don't think it is appropriate to skip, even for markdown, see the reply above: https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2352219690

> There are many other linters that do raise, and even many that only serve to lint docs. If they serve no purpose, they should be removed, instead of being (required to be) skipped.

Which linters only serve to lint documentation-only files? (I could be missing some.)

> Regardless, many doc updates are only checked at compile time (doxy
...

💬 maflcko commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#issuecomment-2353200196)
> (DrahtBot is pinging for a re-review, but the pull hasn't been updated yet.)

This is intentional, because at least one reviewer thinks that no further update is needed and indicated with their review that the change would be fine to merge. The Bot can't evaluate the content of the review comments itself (obviously), so it is up to each reviewer to evaluate whether it can be merged as-is, or not.

💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1761351337)
Will do if I retouch.

💬 jonatack commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#issuecomment-2353234685)
Thanks @maflcko (hm, maybe bump the threshold to 2 new ACKs, or drop the rule in this case).

Friendly ping to @LarryRuane to update.

💬 maflcko commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#issuecomment-2353257205)
> Thanks @maflcko (hm, maybe bump the threshold to 2 new ACKs, or drop the rule in this case).

Pull requests, or issues are welcome, see also https://github.com/maflcko/DrahtBot/issues/33. However, in this case I'd find it useful to be pinged (otherwise I may miss/forget to re-review or otherwise reply).

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2353260451)
> It seems the assertion was [introduced here](https://github.com/bitcoin/bitcoin/pull/29656/files#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98R5556), and my branch doesn't include it.

No, that's just a commit where the variable is renamed. The `Assume` was introduced here: https://github.com/bitcoin/bitcoin/pull/29370/commits/0fd915ee6bef63bb360ccc5c039a3c11676c38e3. The [assumption (pun intended)](https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1479427801)
...

💬 hebasto commented on pull request "build: improve cxx and linker flag caching":
(https://github.com/bitcoin/bitcoin/pull/30732#issuecomment-2353274116)
Adding a source hash to the cache variable name became necessary at some point during the development of the CMake staging branch, as checks for the same flags could occur with different sources in a single `cmake` invocation. Is this scenario possible in the master branch? If not, perhaps this functionality can be dropped?

2a97482f7892ee2b55955850c92ffd985a58c0a0
I'm still not convinced about the last commit, as the situation "when `working_linker_werror_flag` are changed" never occurs duri
...

👍 TheCharlatan approved a pull request: "guix: (explicitly) build Linux GCC with `--enable-cet`"
(https://github.com/bitcoin/bitcoin/pull/30438#pullrequestreview-2307061040)
ACK 89bf11b807252fe5839b5b18742e24568dfe7bbd

Guix build (aarch64)
```
94142f4399e6b57ae5d95364685cf545a20e1974eb3e6061e62b77af57e59a6b guix-build-89bf11b80725/output/aarch64-linux-gnu/SHA256SUMS.part
1bbbbc9c2818eb6c87a7d7164afc61eee299d8417d986832eb0d503484994f4b guix-build-89bf11b80725/output/aarch64-linux-gnu/bitcoin-89bf11b80725-aarch64-linux-gnu-debug.tar.gz
0601f57454694181473f03bc8ff6c6f23f6ff50f38190a9ff42bd351ff739460 guix-build-89bf11b80725/output/aarch64-linux-gnu/bitcoin-89
...

💬 maflcko commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2353289836)
> I am thinking if this should still be fixed in v28, so that users don't see this scary `Internal bug detected` log, what do you think @maflcko ?

Yeah, it should probably be fixed or worked around in v28. But AU is marked experimental (if it isn't it should be done, for at least one release), so a fix for a debug log warning doesn't have to be rushed and can be done for rc3, or even 28.1, imo.

💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#issuecomment-2353296446)
utACK fa99e4521b6fc0e7f6636d40bc0d6a7325227374

💬 fanquake commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1761424826)
This line can remain: https://github.com/bitcoin/bitcoin/pull/30902#discussion_r1759727845.

💬 l0rinc commented on pull request "build: improve cxx and linker flag caching":
(https://github.com/bitcoin/bitcoin/pull/30732#issuecomment-2353313839)
Thanks for checking @hebasto!

> Is this scenario possible in the master branch? If not, perhaps this functionality can be dropped?

There's only a single case where SOURCE is defined now - in which case the related flags were also unique:
> LINKER_SUPPORTS__FSANITIZE_UNDEFINED_ADDRESS_FUZZER__WL__FATAL_WARNINGS_

We could drop the hash, but that means that whoever calls that method again has to change the implementation, since it's not safe to call it without knowing who else called it,
...

💬 achow101 commented on pull request "qt: Translations update":
(https://github.com/bitcoin/bitcoin/pull/30899#issuecomment-2353314794)
Backported in #30827

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1761429706)
no problem removed in [03fd466](https://github.com/bitcoin/bitcoin/pull/30875/commits/03fd466581666cfd637e79a8594a9a0e7d097524)[03fd466](https://github.com/bitcoin/bitcoin/pull/30875/commits/03fd466581666cfd637e79a8594a9a0e7d097524)[03fd466](https://github.com/bitcoin/bitcoin/pull/30875/commits/03fd466581666cfd637e79a8594a9a0e7d097524)

💬 achow101 commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2353325069)
ACK a240e150e837b5a95ed19765a2e8b7c5b6013f35

💬 fanquake commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2353336442)
https://github.com/bitcoin/bitcoin/blob/37679b856ce183ec256c12a680b93ad53ed94da6/doc/design/libraries.md?plain=1#L11