Bitcoin Core Github
44 subscribers
120K links
Download Telegram
👍 l0rinc approved a pull request: "refactor: Drop deprecated space in operator""_mst"
(https://github.com/bitcoin/bitcoin/pull/31267#pullrequestreview-2427272234)
ACK faf21625652fd0d4bbf9b86fd9ebedb5857505ea
💬 l0rinc commented on pull request "refactor: Drop deprecated space in operator""_mst":
(https://github.com/bitcoin/bitcoin/pull/31267#discussion_r1836659165)
nit: given the focus on miniscript here, we might as well use the whole namespace like we do for e.g. `std::literals`:
```suggestion
using namespace miniscript;
```
💬 l0rinc commented on pull request "refactor: Drop deprecated space in operator""_mst":
(https://github.com/bitcoin/bitcoin/pull/31267#discussion_r1836663735)
nit: `Fragment` seems unused
💬 BrandonOdiwuor commented on pull request "doc: Fix gen-manpages to check build options":
(https://github.com/bitcoin/bitcoin/pull/29457#discussion_r1836669430)
@maflcko, are you suggesting the default behaviour should be to also fail if the `enabled components` are missing?
💬 fanquake commented on pull request "refactor: Drop deprecated space in operator""_mst":
(https://github.com/bitcoin/bitcoin/pull/31267#issuecomment-2468185362)
> Clang-20 will do so by default, according to https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated-literal-operator.

Checked that main is doing this:
```bash
In file included from /bitcoin/src/test/miniscript_tests.cpp:17:
/bitcoin/src/script/miniscript.h:153:34: warning: identifier '_mst' preceded by whitespace in a literal operator declaration is deprecated [-Wdeprecated-literal-operator]
153 | inline consteval Type operator"" _mst(const char* c, size_t l) {
|
...
💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1836690281)
Still rebooting :brain: for this week, sorry for the noise.
💬 maflcko commented on pull request "refactor: Drop deprecated space in operator""_mst":
(https://github.com/bitcoin/bitcoin/pull/31267#discussion_r1836692475)
An explicit namespace can avoid ambiguity and naming collisions, or inconsistencies arising from it, so I'll leave this as-is for now. `std::literals` is different, because there can't be any collisions (as explained in the first link I provided in the pull request description).
💬 maflcko commented on pull request "refactor: Drop deprecated space in operator""_mst":
(https://github.com/bitcoin/bitcoin/pull/31267#discussion_r1836698416)
May do if I retouch, or if clang-tidy is patched to find this. Ref:

```
$ git grep 'using' src/.clang-tidy
src/.clang-tidy:misc-unused-using-decls,
💬 brunoerg commented on pull request "fuzz: fix bad alloc in connman target":
(https://github.com/bitcoin/bitcoin/pull/31235#discussion_r1836702319)
> I think fuzzing in range between 0 and 100 would make sense - after all that's what the function is designed to handle

I agree.
💬 fanquake commented on pull request "test: clarify log messages when handling SOCKS5 proxy connections":
(https://github.com/bitcoin/bitcoin/pull/31239#issuecomment-2468267372)
> While we're updating this file,

Pulled this into another branch.
🚀 fanquake merged a pull request: "test: clarify log messages when handling SOCKS5 proxy connections"
(https://github.com/bitcoin/bitcoin/pull/31239)
fanquake closed a pull request: "doc: fix typos"
(https://github.com/bitcoin/bitcoin/pull/31253)
💬 fanquake commented on pull request "doc: fix typos":
(https://github.com/bitcoin/bitcoin/pull/31253#issuecomment-2468268588)
Pulled this into another branch.
👍 fanquake approved a pull request: "doc: Add missing 'blank=true' option in offline-signing-tutorial.md"
(https://github.com/bitcoin/bitcoin/pull/31237#pullrequestreview-2427391108)
ACK ec375de39ff8e0d44fc026618a234e37019e46c1
🚀 fanquake merged a pull request: "doc: Add missing 'blank=true' option in offline-signing-tutorial.md"
(https://github.com/bitcoin/bitcoin/pull/31237)
📝 fanquake locked a pull request: "GitHub skip branch"
(https://github.com/bitcoin/bitcoin/pull/31265)
Testing https://github.com/bitcoin/bitcoin/pull/30487
📝 fanquake opened a pull request: "doc: correct typos"
(https://github.com/bitcoin/bitcoin/pull/31271)
Includes #31253.
Includes https://github.com/bitcoin/bitcoin/pull/31239#pullrequestreview-2425008603.
Fixes remaining lint output.
💬 fanquake commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2468286812)
> Avoid having many static libraries

Can you expand the motivation.
👍 dergoegge approved a pull request: "validation: Remove RECENT_CONSENSUS_CHANGE validation result"
(https://github.com/bitcoin/bitcoin/pull/31269#pullrequestreview-2427403124)
ACK e80e4c6ff91e27d7d40f099a2d7942c29085234c

We can always re-introduce the concept and actually make use of it if needed. For now it seems better to remove it from the interface.
💬 fanquake commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#issuecomment-2468288385)
Concept ACK.