💬 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
...
(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.
(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
(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
(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.
(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)
(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.
(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)
(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.
(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
(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`.
(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
(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
(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
(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
(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
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1452381645)
done
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1892185801)
Thanks for the review @andrewtoth!
All nits tackled, [small diff](https://github.com/bitcoin/bitcoin/compare/6c603490023317a84c7c96ef4b64f4f96ea03d1f..79e10351b1ce1f8db98ece67aacc24f323008b37). Ready to go.
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1892185801)
Thanks for the review @andrewtoth!
All nits tackled, [small diff](https://github.com/bitcoin/bitcoin/compare/6c603490023317a84c7c96ef4b64f4f96ea03d1f..79e10351b1ce1f8db98ece67aacc24f323008b37). Ready to go.
💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1892249295)
Rebased on fix for CI issue
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1892249295)
Rebased on fix for CI issue
⚠️ Laughter79 opened an issue: "https://api.substack.com/feed/podcast/375278/private/b6bb76fe-b98c-4862-8885-1b0beb0fb3e4.rss"
(https://github.com/bitcoin/bitcoin/issues/29250)

(https://github.com/bitcoin/bitcoin/issues/29250)
