devtools: Fetch and display ACKs at sign-off time in github-merge #16223

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2019_06_contrib_acks changing 1 files +49 −19

laanwj commented at 12:47 PM on June 17, 2019: member
- Fetch the ACKs only at sign-off time. This makes sure that any last-minute ACKs are included (fixes #16200)
- Show a list of ACKs that will be included and their author before signing off, and warn if there are none
There's a slight change to the merge commit format—before it was
```
    ACKs for commit 88884c:
(list of ACKs, could be empty)
```
now it is
```
ACKs for top commit:
      jnewbery:
        ACK 5ebc6b0eb
    ... (list of ACKs cannot be empty)
```
or
```
Top commit has no ACKs.
```
I don't think there's a reason to have the abbreviated commit ID there, after all the full commit id is already in the beginning of the merge commit message, and at least the abbreviated one is in every single ACK message.
laanwj added the label Scripts and tools on Jun 17, 2019

devtools: Fetch and display ACKs at sign-off time in github-merge

- Fetch the ACKs only at sign-off time. This makes sure that any
  last-minute ACKs are included (fixes #16200)
- Show a list of ACKs and their author before signing off, and warn if
  there are none

0e01e4522e

laanwj force-pushed on Jun 17, 2019
MarcoFalke commented at 3:45 PM on June 17, 2019: member

Concept ACK
Empact commented at 3:47 PM on June 17, 2019: member

Concept ACK
fanquake commented at 7:52 AM on June 18, 2019: member

Concept ACK - Will test shortly.
laanwj commented at 1:12 PM on June 18, 2019: member

This could also check "signed ACKs" (and say, mark them specially, if they match the expected key for the user) if we're going there in the future.
instagibbs commented at 1:54 PM on June 18, 2019: member

@laanwj I was going to suggest that as a stretch goal, so concept ACK that. Would be nice to see local pgp sigs validated.
laanwj commented at 3:32 PM on June 18, 2019: member

FWIW I merged 15651 using this, seems to work as expected:
fanquake commented at 9:58 AM on June 24, 2019: member

ACK 0e01e4522e0f9e2e994f80fc61f40d61a4a70d79

Did some quick testing merging this PR on master (c8fee6769a106bb5fbcdd3674fd0444b01d3391c), then merging another PR on top.

PR with ACKs:

PR with no ACKs: @laanwj can you update the "now it is" section in PR description? As it does not seem to reflect how ACKs are displayed in this PR.
MarcoFalke merged this on Jun 24, 2019
MarcoFalke closed this on Jun 24, 2019
MarcoFalke referenced this in commit e115a21f79 on Jun 24, 2019
fanquake commented at 12:46 AM on June 25, 2019: member

@laanwj @instagibbs Opened #16280 to track the pgp related github-merge additions.
sidhujag referenced this in commit 107fd6235d on Jun 30, 2019
fanquake referenced this in commit b58763ba01 on Jul 7, 2019
PastaPastaPasta referenced this in commit d200c8e1bf on Sep 11, 2021
PastaPastaPasta referenced this in commit 7b046505a5 on Sep 11, 2021
PastaPastaPasta referenced this in commit ede3de195d on Sep 12, 2021
MarcoFalke locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#16200 github-merge: fetch ACKS and show them before signing off #16280 scripts: verify pgp signatures as part of github-merge.py