💬 maflcko commented on pull request "logging: Update to new logging API":
(https://github.com/bitcoin/bitcoin/pull/29231#issuecomment-1891833356)
> happy to open a separate pull for it too?

Yeah, I think it is better to change a line of code only once, instead of twice, unless there is a reason to change it twice.

💬 torkelrogstad commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#issuecomment-1891915104)
I believe CI failures are unrelated. Is it possible to force CI to re-run?

💬 torkelrogstad commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1891924076)
> However, it is quite convenient to run `./test/lint/all-lint.py` locally, no?

Chiming in as a very inexperienced contributor that has been bashing against CI linter failures: I would say it's very nice to have an easy way to run all tests and checks locally before pushing. The correct thing to do here would be to _expand_ the linter script to include more of the checks, IMO.

On a slightly related note: it would also be nice to include either instructions or CLI flags that fix linter com
...

⚠️ fanquake opened an issue: "build: multiprocess compile failure on macOS"
(https://github.com/bitcoin/bitcoin/issues/29248)
master @ 28ccc7003a4c3f198a255d6f55f5748d4fd8fd1a

```bash
Apple clang version 15.0.0 (clang-1500.1.0.2.5)

make -C depends -j12 MULTIPROCESS=1 NO_QT=1 NO_WALLET=1 NO_NATPMP=1 NO_UPNP=1 NO_ZMQ=1
./autogen.sh
CONFIG_SITE=/bitcoin/depends/aarch64-apple-darwin23.2.0/share/config.site ./configure
make
```

```bash
In file included from ipc/capnp/echo.capnp.proxy-client.c++:3:
In file included from ./ipc/capnp/echo.capnp.proxy-types.h:6:
In file included from ./ipc/capnp/echo.capnp.prox
...

💬 maflcko commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1891989227)
> The correct thing to do here would be to _expand_ the linter script to include more of the checks, IMO.

If you want to run all checks locally, you can refer to the documentation (https://github.com/bitcoin/bitcoin/blob/master/test/lint/README.md#running-locally). The options include running them in a docker container, or via the test_runner.

I am happy to close this pull, but `all-lint.py` never was a way to run "all" lint checks (the above mentioned were always excluded) and given that
...

💬 maflcko commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1891992233)
> I had issues with getting the whitespace linter to pass, and couldn't find any Makefile target, script or similar to automatically format my Python code.

I think you can use any automated formatter of your choice. For example `black` (for a file that was written fully fresh), or yapf-diff (see the docs in this repo), or any other automated formatter.

💬 TheCharlatan commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1892022330)
Concept ACK

💬 maflcko commented on pull request "wallet: BIP 326 sequence based anti-fee-snipe for taproot inputs":
(https://github.com/bitcoin/bitcoin/pull/24128#issuecomment-1892024499)
rebased

💬 furszy commented on pull request "doc, test: test and explain service flag handling":
(https://github.com/bitcoin/bitcoin/pull/29213#discussion_r1452346405)
It is not immediately clear that additional services flags may be added, but the return value could still be false. And the same seems to happen for the `AddrInfo` timestamp. It can also be updated while the return value remains false.

🚀 fanquake merged a pull request: "depends: Allow PATH with spaces in directory names."
(https://github.com/bitcoin/bitcoin/pull/29237)

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1892167852)
OK I added a commit to change default cookie perms to `400`, and rebased on master to try and get the unrelated CI failure to go away.

✅ fanquake closed a pull request: "build: Fix LTO functionality"
(https://github.com/bitcoin/bitcoin/pull/28876)

💬 fanquake commented on pull request "build: Fix LTO functionality":
(https://github.com/bitcoin/bitcoin/pull/28876#issuecomment-1892168841)
> FWIW, the https://github.com/bitcoin/bitcoin/pull/29185 makes this PR outdated.

Closing this for now, given it looks like we are going ahead with #29185.

💬 1440000bytes commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1892179472)
I think [consensus ](https://github.com/bitcoin/bitcoin/labels/Consensus) label should be added for this pull request

📝 fanquake opened a pull request: "depends: add NM output to gen_id"
(https://github.com/bitcoin/bitcoin/pull/29249)
`NM` is part of the current toolset, and can be set by the user. Include it in `gen_id`.

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1452381557)
done

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1452381462)
done

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1452381283)
done

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1452381691)
done

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1452381645)
done