scripts and tools: Fetch missing review comments in github-merge.py #15838

pull nkostoulas wants to merge 1 commits into bitcoin:master from nkostoulas:2019-04-fix-merge-script changing 1 files +30 −11
  1. nkostoulas commented at 4:26 PM on April 17, 2019: contributor

    Use GitHub API pagination to do multiple requests if required.

    Tested with some PRs that have a large number of comments.

    For issue #15816

  2. in contrib/devtools/github-merge.py:69 in 26edc9f477 outdated
      69 | +            return json.load(reader(get_response(req_url, ghtoken)))
      70 | +
      71 | +        obj = []
      72 | +        page_num = 1
      73 | +        while True:
      74 | +            req_url_page = '%s?page=%d' % (req_url, page_num)
    


    MarcoFalke commented at 6:35 PM on April 17, 2019:

    Style-nit: Could use str.format

                req_url_page = '{}?page={}'.format(req_url, page_num)
    
  3. in contrib/devtools/github-merge.py:75 in 26edc9f477 outdated
      75 | +            result = get_response(req_url_page, ghtoken)
      76 | +            obj.extend(json.load(reader(result)))
      77 | +
      78 | +            link = result.headers.get('link', None)
      79 | +            if link is not None:
      80 | +                links = link.split(',')
    


    MarcoFalke commented at 6:50 PM on April 17, 2019:

    style-nit: Could filter right here and get rid of the found bool?

                    link_next = [l for l in link.split(',') if 'rel="next"' in l]
    

    nkostoulas commented at 8:19 PM on April 17, 2019:

    I rembered the bool was to avoid going through the whole list but I changed it anyway.

  4. in contrib/devtools/github-merge.py:79 in 26edc9f477 outdated
      79 | +            if link is not None:
      80 | +                links = link.split(',')
      81 | +                found = False
      82 | +                for link in links:
      83 | +                    if 'rel="next"' in link:
      84 | +                        page_num = int(link[link.find("page=") + 5])
    


    MarcoFalke commented at 6:51 PM on April 17, 2019:

    comment: Doesn't work for double-digit page numbers, but fine

  5. MarcoFalke approved
  6. MarcoFalke commented at 6:51 PM on April 17, 2019: member

    utACK , feel free to ignore the comments

  7. contrib: gh-merge: Use pagination to fetch all review comments 942ff2054b
  8. nkostoulas force-pushed on Apr 17, 2019
  9. DrahtBot added the label Scripts and tools on Apr 17, 2019
  10. MarcoFalke commented at 9:20 PM on April 17, 2019: member

    utACK 942ff2054b41fe3f78f1b3d88cfd032bc95fd62f

  11. DrahtBot commented at 3:14 PM on April 18, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  12. laanwj commented at 5:54 PM on April 18, 2019: member

    utACK 942ff2054b41fe3f78f1b3d88cfd032bc95fd62f

    I guess this makes it even more important to use an API token to not run into request limits.

  13. laanwj merged this on Apr 18, 2019
  14. laanwj closed this on Apr 18, 2019

  15. laanwj referenced this in commit 2d4f70cabd on Apr 18, 2019
  16. fanquake referenced this in commit 5ff91d2bfd on Jul 7, 2019
  17. PastaPastaPasta referenced this in commit 02b6ff089e on Sep 11, 2021
  18. PastaPastaPasta referenced this in commit 6f4ebec66a on Sep 11, 2021
  19. PastaPastaPasta referenced this in commit 7e5260568f on Sep 12, 2021
  20. DrahtBot locked this on Dec 16, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-14 21:14 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me