Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 maflcko commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2250784148)
can be resolved?
💬 maflcko commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2250792650)
could just remove it?
💬 maflcko commented on pull request "tests: cover abortrescan() in-flight True path with dynamic-tail retry":
(https://github.com/bitcoin/bitcoin/pull/33131#discussion_r2250797712)
why not just use the maximum in the first try?
💬 maflcko commented on pull request "ci: Use mlc `v1` and ignore `depends/work`":
(https://github.com/bitcoin/bitcoin/pull/33125#discussion_r2250809067)
This was already reported in https://github.com/bitcoin/bitcoin/issues/31044 (and fixed?) So it looks like a regression and this is the wrong fix?
💬 ismaelsadeeq commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2250835852)
Yes
💬 hebasto commented on issue "OpenBSD, NetBSD: `-reindex` is broken":
(https://github.com/bitcoin/bitcoin/issues/33128#issuecomment-3149692620)
> Does this mean that `feature_reindex.py` still succeeds, so that reindex only fails if we delete the `index/` dir like in `feature_reindex_init.py`, but not if we just reindex without corrupting anything?!

Correct.
💬 dergoegge commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3149703053)
> to better support forked clients

Concept NACK

This is not something Bitcoin Core needs to support.
💬 fanquake commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2250868861)
Are we going to pull in the updates, or leave this as-is?
🤔 janb84 reviewed a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33094#pullrequestreview-3083347788)
ACK 53461dc5cedf5f7080c09246ef76ba8a6c38a103

This PR refactors the code to use the more modern contains() method. In my opinion this PR increased the readability of the code and removes the ambiguity of the intention of the count() methods used. With this change, the intent to enforce that exactly one item is/ is not present or just a presence check will be more obvious. (count() vs contains()).

The argument NOT to change the code because of the risk losing the original behaviour is an
...
💬 maflcko commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2250943593)
> Are we going to pull in the updates, or leave this as-is?

I've also created a preliminary review for all languages: https://github.com/maflcko/b-c-gui-translations-review/tree/main/reviews

There is some obvious erroneous translations, like https://github.com/maflcko/b-c-gui-translations-review/blob/05100fec918fc44787d43004ea3b02749f5a16ee/reviews/te.md#L160-L166, but I am not familiar with those languages, so I can't evaluate if the LLM generated translation is correct.
💬 janb84 commented on pull request "doc: gen-manpages.py should check build options":
(https://github.com/bitcoin/bitcoin/pull/33085#issuecomment-3149866089)
Personally I like the intent to structure the code more (the introduction of functions).

Paused reviewing this PR:
- I paused reviewing this PR because the way the code is currently structured is unfinished to me. The introduction of a function for the new functionality is great but the rest of the code needs to be restructured to better incorporate the use of functions imho (e.g. use a main function).
- Have not looked at the intent of the code yet.

(P.S. This is not meant to be har
...
💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251017723)
Agree. I removed this "optimisation" in eb68bb48ab2bb163f20dc7430d0401d88ca43f4a

We now just check both directories to avoid missing any as you describe (and if they are renamed as per https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2171706368 )
💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251019297)
Going to leave this nit, as I don't think it's worth exatra logic here
💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251021307)
FWIW it is now working for GUI startup according to https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-2829171034 in eb68bb48ab2bb163f20dc7430d0401d88ca43f4a
💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251022089)
MacOS corrected to macOS in according to #31453 (comment)
💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2251024649)
I've avoided explicit recommendation of a FS on purpose here; as far as I know exFAT is the only known-problematic FS on macOS?

Can be convinced otherwise, but leaving as-is for now
💬 willcl-ark commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-3149949745)
While I was re-touching this for the outstanding comments I took the chance to inline the check for a smaller diff and generally a little cleaner/better code (IMO).
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2251119754)
Removed in dc1613169db
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3150096441)
Pushed another run with caches saving to re-test cache hit/restore later.
👍 stickies-v approved a pull request: "kernel: create monolithic kernel static library"
(https://github.com/bitcoin/bitcoin/pull/33077#pullrequestreview-3083677951)
tACK fdbade6f8ded63519b637c8fa6f39914a400ab5e

I rebased https://github.com/TheCharlatan/bitcoin/commits/kernelApi_Cpp_Internal_Headers/ onto fdbade6f8ded63519b637c8fa6f39914a400ab5e (resulting in https://github.com/stickies-v/bitcoin/tree/kernel-cpp-monolith), and I was able to statically link my `pbk_ext` py-bitcoinkernel nanobind extension to the monolithic libbitcoinkernel.

I reviewed the code changes and could not see any issues, but I am also a cmake novice. The monolithic approach ma
...