💬 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

💬 purpleKarrot commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#discussion_r2282215658)
This also sets the [policy version](https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html#policy-version). No matter what version of CMake you use, it will behave as if you used 3.22.

🤔 janb84 reviewed a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3128106813)
Concept ACK d3679017c64f336f94339373e563bd4967cd143c

PR moves functionality from python based script to cmake, which imho better aligns with the place from where the script is called (from a build) and it removes an external dependency.

- build ✅
- followed translation instruction that calls the script ✅