💬 fanquake commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349066694)
At first glance, I'm not sure why we'd want to start skipping commits, in a CI job, where the entire purpose is to test all commits.

💬 maflcko commented on pull request "kernel: Move background load thread to node context":
(https://github.com/bitcoin/bitcoin/pull/30896#issuecomment-2349069247)
ACK bc7900f33db3d01fb93dfee7981c01ea495cd42e 🔄

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK bc7900f33db3d01fb93dfee798
...

💬 Sjors commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349074587)
@fanquake sometimes a large refactor has intermediate commits that don't compile or fail the test. This should be discouraged, but I don't think it should be impossible. We have multiple `[ci skip]` and `[skip ci]` commits already in the repo.

💬 maflcko commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349078545)
> This should be discouraged, but I don't think it should be impossible.

This happens already today, with no further changes needed. The CI task will turn red to discourage it, but it is not impossible to review or merge the pull request.

👍 ryanofsky approved a pull request: "kernel: Move background load thread to node context"
(https://github.com/bitcoin/bitcoin/pull/30896#pullrequestreview-2303276909)
Code review ACK bc7900f33db3d01fb93dfee7981c01ea495cd42e. Nice cleanup

👍 ryanofsky approved a pull request: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block"
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2303280869)
Code review ACK 76ec301c60e2530de7f58e2e4cea3d15c6a77cbc. Just simple rebase since last review

💬 Sjors commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349086465)
#19982 is an example of a PR which had an intermediate commit marked `[skip ci]`. Unfortunately I can't see how the CI handled that back then.

👍 ryanofsky approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2303285049)
Code review ACK a93c171faa7b4426823466e972c8f24260918bbf. Since last review, just rebased with no changes or conflicts

🚀 fanquake merged a pull request: "build: add `standard branch-protection` to hardening flags for aarch64-linux"
(https://github.com/bitcoin/bitcoin/pull/30433)

💬 maflcko commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349100068)
1 commit every 4 years, or even 1 commit every year that qualifies for this just doesn't seem worth it the overhead.

If you want to break `git bisect`, there should be a good reason. A reason well enough that reviewers run the appropriate `git rebase --interactive --exec` locally. If there is no good enough reason, then the breaking change probably shouldn't be done.

💬 Sjors commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349119854)
> A reason well enough that reviewers run the appropriate git `rebase --interactive --exec` locally.

That seems a fairly arbitrary metric for measuring how good a reason is.

I'm more sympathetic to the possibility that both the reviewers and maintainer overlook a `[skip ci]` commit message (especially if it's not on the first line), causing `git bisect` to break. And worse, potentially allowing to sneak in a non-refactoring change.

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758992957)
fixed

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993050)
good idea, moved this subtest to `mempool_dust.py` as its own commit, switched it to testing dustrelay arg instead of standardness as I think that's more directly testing what we want.

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993149)
dropped a note

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993261)
added suggestion and a direct assertion that the main output amount is >330 the taproot dust amount

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993345)
dropped the reference to modified as it probably isn't clarifying

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993386)
Yes, if it's fixed this will fail, letting the author know to write a new case. Added a note that this isn't desired, just descriptive,

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993580)
would cause the nodes to fail to sync their mempools, no? I find it easiest when the restarts are done in sync anyways.

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993663)
added a sentence to each saying when they should be called

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993726)
taken