💬 Galoretka commented on pull request "doc: add Linux GUI runtime instructions to doc/README.md":
(https://github.com/bitcoin/bitcoin/pull/33197#discussion_r2282011086)
> the minimum required is 6.2? Why is this duplicated here at all?

really, removed copying

💬 hebasto commented on something "":
(https://github.com/bitcoin/bitcoin/commit/d5054beca50f992dc63dc596f0aff457eb9d217f#commitcomment-164149256)
Suggesting a few cahnages:
```diff
--- a/share/qt/translate.cmake
+++ b/share/qt/translate.cmake
@@ -9,7 +9,6 @@ set(input_variables
COPYRIGHT_HOLDERS
LCONVERT_EXECUTABLE
LUPDATE_EXECUTABLE
- PYTHON_EXECUTABLE
XGETTEXT_EXECUTABLE
)

@@ -29,28 +28,24 @@ function(extract_strings output)
--from-code=utf-8
--keyword=_
${ARGN}
- OUTPUT_VARIABLE text
COMMAND_ERROR_IS_FATAL ANY
)

- set(messages)
- set(msgid)
+ set(messages "")
+
...

💬 maflcko commented on pull request "doc: add Linux GUI runtime instructions to doc/README.md":
(https://github.com/bitcoin/bitcoin/pull/33197#discussion_r2282036850)
is the goal of this change to fix the issue?

💬 maflcko commented on pull request "cmake: Do not require Python to build GUI":
(https://github.com/bitcoin/bitcoin/pull/33156#issuecomment-3196146326)
> Add to the 30.0 milestone?

This one or https://github.com/bitcoin/bitcoin/pull/33200?

👍 hebasto approved a pull request: "cmake: Introduce translate.cmake script for translate target"
(https://github.com/bitcoin/bitcoin/pull/33200#pullrequestreview-3127870644)
ACK 05255d5d1ec1852d8d8d7683ccbf28351f57b89e.

🚀 hebasto merged a pull request: "cmake: Introduce translate.cmake script for translate target"
(https://github.com/bitcoin/bitcoin/pull/33200)

💬 fanquake commented on pull request "cmake: Introduce translate.cmake script for translate target":
(https://github.com/bitcoin/bitcoin/pull/33200#issuecomment-3196172145)
Would have been good to update the PR description before merge? It says "As a follow up, the dependency on sed may be removed ..." but it looks like that was done here?

💬 hebasto commented on pull request "cmake: Do not require Python to build GUI":
(https://github.com/bitcoin/bitcoin/pull/33156#issuecomment-3196203274)
> > Add to the 30.0 milestone?
>
> This one or #33200?

#33200 doesn't solve the issue. But @purpleKarrot has a working branch on top of it that replaces Python code with pure CMake code.

📝 purpleKarrot opened a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209)
Translate the `share/qt/extract_strings_qt.py` script to CMake. This removes the python dependency from the `translate` target.

💬 purpleKarrot commented on pull request "cmake: Do not require Python to build GUI":
(https://github.com/bitcoin/bitcoin/pull/33156#issuecomment-3196219744)
This removes the Python dependency: https://github.com/bitcoin/bitcoin/pull/33209

👋 hebasto's pull request is ready for review: "Release: Prepare "Translation string freeze" step"
(https://github.com/bitcoin/bitcoin/pull/33193)

💬 hebasto commented on pull request "Release: Prepare "Translation string freeze" step":
(https://github.com/bitcoin/bitcoin/pull/33193#issuecomment-3196246588)
Rebased on top of the merged #33200 and undrafted.

💬 maflcko commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#discussion_r2282116087)
```suggestion
# We need to temporarily replace that in order to keep strings intact.
```

(nit from the llm)

💬 maflcko commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#discussion_r2282115116)
Can be dropped, given the global requirement in the root `CMakeLists.txt`?

💬 maflcko commented on pull request "cmake: Do not require Python to build GUI":
(https://github.com/bitcoin/bitcoin/pull/33156#issuecomment-3196255679)
I see. No strong opinion on which direction to take, but this one need rebase, if still relevant.

💬 purpleKarrot commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#discussion_r2282132313)
No. This is a script that is executed as a separate process. It does not inherit any settings from the buildsystem generator invocation.

💬 purpleKarrot commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#discussion_r2282133378)
Thanks.

💬 maflcko commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#discussion_r2282164552)
Just a nit, but I mentioned it, because we don't add it to any other script (cmake, python, bash, ...). They are simply documented in `doc/dependencies.md` and bumping them is done with a minimal patch, not touching all possible scripts that could be called as an entrypoint. Also, this protects against an edge case, that doesn't exist in reality: It requires a developer that uses an ancient cmake to call this entrypoint manually. But just a nit.

💬 maflcko commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33094#issuecomment-3196344455)
It is already possible to run clang tidy on a diff. See `git grep clang-tidy-diff`. However, it is not integrated into CI. In theory it could be integrated, enforcing a stronger set of checks than the "default" ones. Though, that's also going to mess with move-only commits, so I am not sure if it will be useful to have it enabled and lead to failing CI. I'd say there are two options:

* Enable it globally and by default in a "breaking" CI change that may lead to silent or explicit merge confli
...

💬 maflcko commented on pull request "test: cover invalid codesep positions for signature in taproot":
(https://github.com/bitcoin/bitcoin/pull/32301#issuecomment-3196369804)
CI failure:

```
[08:01:28.655] �[0;36m OverflowError: can't convert negative int to unsigned�[0m