Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 dergoegge commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2313212435)
The two comments are saying the same thing, I'd suggest to just keep one
💬 maflcko commented on issue "intermittent timeout in mptest unit test":
(https://github.com/bitcoin/bitcoin/issues/33244#issuecomment-3241343804)
ctest doesn't have a default timeout, so it would be a bit odd to expose users to a unit test run that never finishes, albeit rare?

> this issue was more artificial, happening as a result of the way the test was written.


This feature is experimental anyway, so maybe the unit test could be rewritten or removed temporarily for the 30.x release branch, if fixing it is too invasive for now?
💬 Sjors commented on issue "intermittent timeout in mptest unit test":
(https://github.com/bitcoin/bitcoin/issues/33244#issuecomment-3241353079)
Or we could add a timeout for this specific test. I don't think we should remove it, because we want to catch unknown issues on platforms / circumstances that our CI doesn't cover.
💬 maflcko commented on issue "intermittent timeout in mptest unit test":
(https://github.com/bitcoin/bitcoin/issues/33244#issuecomment-3241422055)
> add a timeout

Sure, but I'd say the timeout should be added in the C++ code, not in ctest, possibly with an error message explaining the known issue.
💬 hebasto commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3241549612)
> I've done some experimenting with a test project on transifex to see what the actual effects this change would cause, and a potential workaround.
>
> In this test project, I copied over a couple fully translated languages to observe how the translated strings change when the source is updated. After rebasing this PR branch and doing `cmake --build build --target translate`, I uploaded the resulting modified `bitcoin_en.xlf` file. This resulted in 122 (~10% of strings) being marked untransla
...
💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2313401875)
Could check the return code and print something to clarify this is an error, similar to the tidy error above:

```sh
echo "^^^ ⚠️ Failure generated from clang-tidy"
false
```
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2313427999)
Merged these into a single step in fdf64e55324, as there was no clear distinction between the two steps.
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2313428760)
amedned in cc1735d7771
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2313429763)
Yes I'd like to keep this for a followup for now.
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2313430468)
Done, here and elsewhere
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3241633101)
Rebased on master to catch upstream changes, manually added changes to align with #31802 and #33099 which were merged while I was away. Hopefully I didn't miss any others.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3241642016)
`dbd76e68c3...2bd4714fa9`: rebase due to conflicts and address https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2274730057

I am back from some afk time.
💬 hebasto commented on pull request "stabilize translations by reverting old ids by text content":
(https://github.com/bitcoin/bitcoin/pull/33270#issuecomment-3241645029)
Since we are not using [ID-based](https://doc.qt.io/qt-6/linguist-id-based-i18n.html) translations, it seems reasonable to consider removing the `id` attributes from `trans-unit` elements in the XML file.
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3241685301)
Just some style fixups, but it looks like out-of-storage errors are happening:

```
/home/admin/actions-runner/_work/_temp/depends/work/build/arm-linux-gnueabihf/qt/6.7.3-634ec665c7f/qtbase/src/gui/util/qvalidator.h:163:2: fatal error: cannot write PCH file: No space left on device
```

re-ACK 8a7eb6ef2c226459739bdfe050b735a5a0d0b771 🕧

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisi
...
💬 maflcko commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3241685779)
To keep a list of follow-ups:

* https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290159660 (merge-base issue)
* https://github.com/bitcoin/bitcoin/pull/33145 (silent merge conflict check)
* https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3234350722 (qa-assets pull)
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3241834149)
Rebased and addressed @maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2313401875).
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2313578680)
Thanks! [Done](https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3241834149).
💬 maflcko commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-3241842331)
Are you still working on this?
💬 hebasto commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3241930685)
> If this doc change is desired by contributors, and the 9 acks suggested that it is, then...

... it can be accepted as is, but left untranslated until the version v31.0.

This is acceptable because:
1. We never promised, nor have we ever delivered, 100% translations for every language.
2. It's still easy for the user to translate the warning message themself.

I still hesitant about introducing last-minute changes to the translation pipeline / framework. We can revisit all related sugg
...