Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 Sjors commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#issuecomment-3062249324)
I missed a few `__func__`. Replaced `LogPrintf` with `LogInfo` and removed trailing `\n`'s
💬 dergoegge commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#issuecomment-3062262136)
While we're touching all of these, it might be worthwhile to change to `LogWarn/LogError` where appropriate.
💬 Sjors commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#issuecomment-3062269312)
@dergoegge any suggestions? Despite the new guidance, it's still not always obvious what log level to use.
💬 dergoegge commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#discussion_r2200688205)
These can all be `LogError`
💬 dergoegge commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#discussion_r2200688995)
LogError
💬 dergoegge commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#discussion_r2200689580)
```suggestion
LogWarn("previous block header belongs to unexpected block %s; expected %s",
```
💬 dergoegge commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#discussion_r2200685338)
```suggestion
LogWarn("Block %s does not connect to an ancestor of "
```
💬 dergoegge commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#discussion_r2200690005)
```suggestion
LogWarn("previous block header belongs to unexpected block %s; expected %s",
```
💬 dergoegge commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#discussion_r2200685734)
```suggestion
LogWarn("Locator contains block (hash=%s) not on known best "
```
💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3062282839)
> What the comment does seem to imply to me is that going forward all new binaries should be put into /libexec and be used through the wrapper binary to avoid idiosyncrasies.

Thanks for the feedback! Just to clarify, I would not say that all new binaries should be put in libexec. I'd just say that unless there is a good reason for a new binary to be installed in `bin/` and present in `PATH` it should be installed in `libexec/` instead. If we developed a new utility that was used by bitcoin co
...
💬 Sjors commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#issuecomment-3062312627)
Thanks for the suggestions @dergoegge.

I replaced:
- `LogInfo("WARNING: ` with `LogWarning`,
- `LogInfo("Failed` with `LogError`
💬 dergoegge commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#discussion_r2200728836)
I think this is also worth `LogError` as it'll require user action.
👍 dergoegge approved a pull request: "log: Properly log warnings with warn loglevel in addrdb"
(https://github.com/bitcoin/bitcoin/pull/32933#pullrequestreview-3010299469)
utACK fa894b0f3e13dcc55fd42cec2c56d4aa2115194d
👍 fanquake approved a pull request: "cmake: Use newer signature of `qt6_add_lrelease` when available"
(https://github.com/bitcoin/bitcoin/pull/32940#pullrequestreview-3010307685)
ACK 94931656b52fddb7c03b29685844d85c008cf6ca
🚀 fanquake merged a pull request: "log: Properly log warnings with warn loglevel in addrdb"
(https://github.com/bitcoin/bitcoin/pull/32933)
fanquake closed an issue: "qt: translation related warnings"
(https://github.com/bitcoin/bitcoin/issues/32710)
🚀 fanquake merged a pull request: "cmake: Use newer signature of `qt6_add_lrelease` when available"
(https://github.com/bitcoin/bitcoin/pull/32940)
💬 Sjors commented on pull request "Enable `-Werror=dev` in CI & Guix":
(https://github.com/bitcoin/bitcoin/pull/32937#issuecomment-3062371486)
re-ACK 8f766f39df3e312f79f461b2accc2f9c90fa6338

Guix build on x86_64:

```
f73822833d8c3ff2b2dda2ddc5e16cb1794aa4eb299b1f468fa8dbadfd6b2310 guix-build-8f766f39df3e/output/aarch64-linux-gnu/SHA256SUMS.part
35783f79defa8b4032b78443a28e88a9c11314470fc177afa9e21d0e613bd937 guix-build-8f766f39df3e/output/aarch64-linux-gnu/bitcoin-8f766f39df3e-aarch64-linux-gnu-debug.tar.gz
b0345a28e82514d51cb363d798d950e44e630ba357a5e522537e1a7b4a37a151 guix-build-8f766f39df3e/output/aarch64-linux-gnu/bitc
...
💬 Sjors commented on issue "Guix build fails on M4 macOS host with Ubuntu in UTM":
(https://github.com/bitcoin/bitcoin/issues/32759#issuecomment-3062401151)
I created a branch to debug this: https://github.com/Sjors/bitcoin/commits/2025/07/guix-aarch64-debug/

It starts from https://github.com/bitcoin/bitcoin/commit/c8abd972818f83d90bf50b250131f338034460ef. First it drops `python-tomli` and `lief` and disables the security checks. I run `HOSTS="arm64-apple-darwin" contrib/guix/guix-build` and make it succeeds.

Next I add `python-scikit-build-core` and pretend its needed for `signapple`. Depending on whether that succeeds, I'll add `python-pydantic-
...
💬 maflcko commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#issuecomment-3062402918)
rebased to drop merged commits and included some minimal fixups