💬 willcl-ark commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541662001)
I agree, but going to leave that from this PR for now.

💬 willcl-ark commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541662319)
Taken in 75155c073d7

💬 willcl-ark commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541662701)
Taken in 75155c073d7

💬 willcl-ark commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541662993)
Taken in 75155c073d7

👍 hebasto approved a pull request: "Document compiler configuration for native depends packages"
(https://github.com/bitcoin/bitcoin/pull/33902#pullrequestreview-3482268948)
re-ACK 7dd714ae71fd18eda82ab4b43d4cecc047b87a2d.

💬 willcl-ark commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541664757)
Reolving this based on wording in 75155c073d7.

💬 janb84 commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#issuecomment-3552252907)

> Also, the title seems the wrong place to list all the shortcomings of each CI task. For example, the win-cross does not build the gui tests, should it say 'no-gui-tests'? Or the arm test has `-Wno-error=maybe-uninitialized`, so should the title say 'no-error-uninit'? Some tasks do not have `CI_LIMIT_STACK_SIZE` set, some tasks don't run the previous releases, or the extended functional tests, some have the gui disabled, no task is using gcc-15, ...
>
> Maybe it makes sense to document thi
...

💬 fanquake commented on pull request "init: Split file path handling out of -asmap option":
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3552262328)
Is this still relevant, given you've ACK'd #33770?

💬 kevkevinpal commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2541695322)
is this the reason for the limitation? or are there other reasons?

if its just this couldn't we just get how many commits are in this branch and use it here?

💬 maflcko commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2541710794)
i don't think this heuristic makes sense, because it is already incomplete with the merge of this pull, and it is unclear how to even maintain it going forward. I don't think it is a good use of time to think about every single named argument, whether it can be a string, and whether the string may theoretically contain a `=`.

It would be better to just fully list all named args names in the client. Then, just check if the string starts with the named arg name. If yes, it is a named arg. If not,
...

💬 fanquake commented on pull request "doc: Add `INSTALL.md` to Linux release tarballs":
(https://github.com/bitcoin/bitcoin/pull/33451#discussion_r2541711810)
xcb & xkb libs can be dropped.

💬 fanquake commented on pull request "doc: Add `INSTALL.md` to Linux release tarballs":
(https://github.com/bitcoin/bitcoin/pull/33451#discussion_r2541716526)
I'm not sure we need to link to Qt docs here, given it's listing ~ 25 different things, of which only 2-3 are relevant to our binaries.

💬 theStack commented on pull request "test: add `-alertnotify` test for large work invalid chain warning":
(https://github.com/bitcoin/bitcoin/pull/33893#issuecomment-3552330703)
In addition to `feature_bip68_sequence.py`, it seems that the functional tests `feature_coinstatsindex.py`, `feature_pruning.py` and `mempool_reorg.py` hit this code-path as well (tested experimentally by putting an `assert(false)` into the branch to provoke a crash), most likely all of them in the course of an `invalidateblock` call, though I didn't check in detail. However, none of these use the `-alertnotify` option, so the function `AlertNotify` returns early in those cases.

💬 l0rinc commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2541732485)
See the explanation above:
> timeout-minutes: 360 # Use maximum time, see https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes. Assuming a worst case time of 1 hour per commit, this leads to a --max-count=6 below.

💬 fanquake commented on pull request "doc: Add `INSTALL.md` to Linux release tarballs":
(https://github.com/bitcoin/bitcoin/pull/33451#issuecomment-3552345830)
Should #29977 be closed, if we add these docs? Is 20.04 considered unsupported?

💬 maflcko commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2541763940)
The last commit isn't checked either. Maybe 'test some mid commits'

💬 l0rinc commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2541775452)
But that's just an optimization, since that's checked elsewhere, no need to point it out here, it's not part of reducing "false sense of security"

💬 fanquake commented on issue "depends: Freetype and xcbproto version in depends are too new":
(https://github.com/bitcoin/bitcoin/issues/29977#issuecomment-3552413548)
Note that libxkb, libxcb, and xcb-proto are now all statically linked, after #33537.

💬 maflcko commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2541787905)
Someone could introduce an inverse silent merge conflict, so in theory that is a "false sense of security"

💬 fanquake commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3552469788)
> minimum GCC version for Windows cross-compilation to [13.1](https://doc.qt.io/qt-6.8/supported-platforms.html#windows).

Have you checked that it doesn't work with 12? According to the documentation, Qt 6.7 (which we currently use) doesn't support any of our Linux compilers. i.e GCC 12+: https://doc.qt.io/archives/qt-6.7/supported-platforms.html#linux-x11, or macOS cross-compilation using Clang: https://doc.qt.io/archives/qt-6.7/supported-platforms.html#macos. So its not clear why we would
...