scripts: GitHub merge script is missing review comments #15816

issue fanquake opened this issue on April 15, 2019

fanquake commented at 1:05 AM on April 15, 2019: member
In #15643 the github-merge.py script was updated so that merge commits would include review comments.

However, the script seems to be missing comments. i.e the recently merged #15557 includes:
```
ACKs for commit 184f87:
      jnewbery:
        utACK 184f8785f710d58d9ef82e611591c9cbff5ab89d
    
    Tree-SHA512: fb6542bdfb2c6010e328ec475cf9dcbff4eb2b1a1b27f78010214534908987a5635797196fa05edddffcbcf2987335872dc644a99261886d5cbb34a8f262ad3e
```
Looking at the PR there should also be utACKs from ryanofsky and promag.
fanquake added the label Scripts and tools on Apr 15, 2019
ryanofsky commented at 1:43 AM on April 15, 2019: member

Looking at the code around retrieve_json in github-merge.py, I don't see anything in there that handles pagination.

The code should be checking for Link: <...> rel="next" response headers and sending followup requests if it's supposed to download all review comments. See https://developer.github.com/v3/guides/traversing-with-pagination/#basics-of-pagination
MarcoFalke added the label good first issue on Apr 15, 2019
laanwj referenced this in commit 2d4f70cabd on Apr 18, 2019
fanquake commented at 12:48 AM on April 27, 2019: member

Should have been fixed by #15838.
fanquake closed this on Apr 27, 2019
PastaPastaPasta referenced this in commit 02b6ff089e on Sep 11, 2021
PastaPastaPasta referenced this in commit 6f4ebec66a on Sep 11, 2021
PastaPastaPasta referenced this in commit 7e5260568f on Sep 12, 2021
DrahtBot locked this on Dec 16, 2021

Contributors

fanquake

ryanofsky

Labels

Scripts and tools good first issue

Linked (view graph)

#15557 Enhance `bumpfee` to include inputs when targeting a feerate #15643 contrib: gh-merge: Include ACKs in merge commit #15838 scripts and tools: Fetch missing review comments in github-merge.py