<bitcoin-git>
[bitcoin] andrewtoth opened pull request #26415: rpc,rest: read raw block in getblock verbosity 0 and rest_block bin and hex (master...read-raw-block) https://github.com/bitcoin/bitcoin/pull/26415
<aureleoules>
Since Drahtbot is now open-source, I would like to improve it so that the bot can create a summary comment at the top of every (new) pull request showing some information related to it.
<aureleoules>
The comment would be live-updated with the current (N)ACKs, Concept (N)ACKs, Approach (N)ACKs, Code Review ACKs, Approach (N)ACKs, and Stale ACKs. The comment would also list the conflicting pulls.
<aureleoules>
Whenever a user would push to the PR, raw ACKs will be removed from the summary comment and placed as "stale".
<aureleoules>
This would allow reviewers to quickly glance at the review stage of the PR by looking at the top comment, before scrolling down to the potential hundreds of comments.
<aureleoules>
This would require DrahtBot to comment on every new pull request to claim the first comment.
<aureleoules>
Is this feature something that you would find useful?
<darosior>
Don't know about the ACKs. But wasn't that you who had a script to expand all the hidden comments on the page? That could be useful to have a tiny link in Drahbot's ubiquitous conflict message to expand all comments at once. The PRs where you want to unroll comments almost always have the conflict comment (since they tend to be large), so this
<darosior>
would not need to add a new message to every PR.
<achow101>
willcl_ark: I was planning on closing after 1 month
<achow101>
aureleoules: I think it would be useful to have an ACK tracker
<aureleoules>
darosior: that's me! Though since I wrote the script I found an amazing browser extension that does this (if you alt+click) on the 'Load more comments' button. And many other cool things: https://github.com/refined-github/refined-github if you're interested
amovfx_ has joined #bitcoin-core-dev
<aureleoules>
The conflict message can be found but IMO it still takes time to find it, whereas having it as the first comment is much easier. Other information useful to the PR could be added later such as whether the PR is stale, if there are unaddressed review comments?
<aureleoules>
achow101: you mean as an external tool or in the summary comment as I described?
<achow101>
in the summary comment
<achow101>
if drahtbot could also always be the first comment, that'd be great
<achow101>
sometimes it's slow to post
amovfx_ has quit [Ping timeout: 252 seconds]
zeropoint has joined #bitcoin-core-dev
amovfx_ has joined #bitcoin-core-dev
yanmaani2 has quit [Remote host closed the connection]
yanmaani2 has joined #bitcoin-core-dev
<jon_atack>
aureleoules: nice, I agree that your first-comment ACK summary would be useful
sipsorcery has joined #bitcoin-core-dev
<brunoerg>
aureleoules: interesting idea, especially about the stale after a push
<aureleoules>
great, thanks for the feedback achow101 jon_atack brunoerg
<aureleoules>
the stale acks thing is kouloumos's original idea :)
amovfx has quit [Ping timeout: 246 seconds]
amovfx has joined #bitcoin-core-dev
amovfx has quit [Ping timeout: 255 seconds]
amovfx has joined #bitcoin-core-dev
AaronvanW has quit [Quit: Leaving...]
<vasild>
aureleoules: looks nice
<vasild>
tACK is not mentioned in CONTRIBUTING.md, it is kind of unofficial
amovfx_ has quit [Ping timeout: 260 seconds]
<vasild>
because there are usually less than ~10 ACKs maybe there is no such big need to group them. If you group then you have to consider what to do with all kinds of "semi ACK ...", "almost ACK ..." "I would ACK this if it is blue" "etc ACK and whatnot"
<aureleoules>
The screenshot is not really up-to-date with what I'm going to implement. The ACKs that will be listed are (N)ACKs, Concept (N)ACKs, Approach (N)ACKs, Code Review ACKs, Approach (N)ACKs, and Stale ACKs. But some are aliased such as "crACK", "CR ACK", "re-crACK" for Code Review ACK, or tACK, utACK, reACK, re-ACK for `ACK` for example.
<aureleoules>
usually if someone acks on a condition, such as a comment needs to be addressed, the commit hash is not provided (from what I've seen). So raw ACKs won't be taken into account if there PR HEAD's hash is not provided
<aureleoules>
if the* PR HEAD's hash is not provided
<vasild>
hmm, right
<vasild>
cool stuff :)
<aureleoules>
thanks :)
<bitcoin-git>
[bitcoin] achow101 closed pull request #24975: bench: remove from available_coins with reference, vout size (master...pool_bench) https://github.com/bitcoin/bitcoin/pull/24975
<aureleoules>
Though what I need to worry about are quoted ACKs when replying, someone might quote a Concept ACK or even a raw ACK
<aureleoules>
that should be fixed by only taking into account ACKs that are the first words of a comment, which is usually the case i think?
<aureleoules>
I don't think the maintainer merge-script checks for quoted acks by the way
amovfx_ has joined #bitcoin-core-dev
jarthur_ is now known as jarthur
gnaf has joined #bitcoin-core-dev
Guyver2 has left #bitcoin-core-dev [Closing Window]
<jon_atack>
aureleoules: i'm not sure "comment starts with ACK" is a good enough heuristic, e.g. "This seems to address x, y, z, so Concept ACK for me", but I could be wrong in recalling seeing review comments like that.
<jon_atack>
ah, unless you don't count ACKs without the commit hash
<brunoerg>
aureleoules: Does it stale a "concept ACK" after a push?
jarthur has quit [Read error: Connection reset by peer]
<aureleoules>
Concept ACKs don't need a commit hash, so you're right jon_atack it can't only be the first words of the sentence
<aureleoules>
it should probably be enough to check that the line is not quoted (doesn't start with '>')
jarthur has joined #bitcoin-core-dev
<aureleoules>
so no brunoerg, Concept ACKs won't get stale, only ACKs (also tACK, utACK, reACKs etc)
<brunoerg>
Cool, it's what i was thinking, thanks
amovfx has quit [Ping timeout: 252 seconds]
dongcarl has joined #bitcoin-core-dev
jon_atack has quit [Ping timeout: 252 seconds]
amovfx_ has quit [Ping timeout: 240 seconds]
<bitcoin-git>
[bitcoin] achow101 opened pull request #26416: Initialize optional members of ChainstateManagerOpts to nullopt (master...fix-warnings) https://github.com/bitcoin/bitcoin/pull/26416
sipsorcery has quit [Ping timeout: 264 seconds]
amovfx has joined #bitcoin-core-dev
amovfx has quit [Ping timeout: 260 seconds]
<bitcoin-git>
[bitcoin] achow101 closed pull request #26416: Initialize optional members of ChainstateManagerOpts to nullopt (master...fix-warnings) https://github.com/bitcoin/bitcoin/pull/26416