Skip to content

Use in_reply_to_id to lookup GitHub replies#1401

Open
kemchenj wants to merge 1 commit intodanger:masterfrom
kemchenj:reply_lookup
Open

Use in_reply_to_id to lookup GitHub replies#1401
kemchenj wants to merge 1 commit intodanger:masterfrom
kemchenj:reply_lookup

Conversation

@kemchenj
Copy link

@kemchenj kemchenj commented Nov 13, 2022

Currently, danger's comment replies lookup logic for GitHub is basically guessing, cause at that time GitHub doesn't have a concept for "reply".

The good news is GitHub API has filled this pit:

// https://docs.github.com/en/rest/pulls/reviews#list-reviews-for-a-pull-request
"in_reply_to_id": {
  "description": "The comment ID to reply to.",
  "type": "integer",
  "examples": [
    8
  ]
}

So let's move on with this new right way.

@manicmaniac
Copy link
Member

manicmaniac commented Nov 16, 2022

@kemchenj

Looks really nice 👍
Could you add some tests for this change?

At a glance, there seems no tests for inline comments with replies but this test case looks similar.

danger/spec/lib/danger/request_sources/github_spec.rb

Line 498 in bd48382

it "removes inline comments if they are not included" do

@orta
Copy link
Member

orta commented Nov 16, 2022

Does anyone know when this was added? Older GitHub enterprises may not have this in their API, which is what I'd be wary of

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kemchenj @manicmaniac @orta