-
Notifications
You must be signed in to change notification settings - Fork 477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix compatibility issue with Octokit >= 8 #1479
Fix compatibility issue with Octokit >= 8 #1479
Conversation
expect(@g.client).to receive(:delete_comment).with("artsy/eigen", inline_issue_id_2).and_return({}) | ||
expect(@g.client).to receive(:delete_comment).with("artsy/eigen", main_issue_id).and_return({}) | ||
|
||
v = Danger::Violation.new("Sure thing", true, "Rakefile", 34) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following diff is used as test fixture.
0d303da
to
5cb00a3
Compare
5cb00a3
to
f8c2eb5
Compare
client.create_pull_request_comment(ci_source.repo_slug, ci_source.pull_request_id, | ||
body, head_ref, m.file, position) | ||
body, head_ref, m.file, (Octokit::MAJOR >= 8 ? m.line : position)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Octokit::MAJOR
exists since Octokit v4.0.0 and it is the minimum version that Danger supports.
Line 31 in bd89c4d
spec.add_runtime_dependency "octokit", ">= 4.0" |
f8c2eb5
to
bf46822
Compare
@orta Could you give me a quick review on this one? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, seems reasonable - feel free to self-merge things you think aren't controversial, and make deploys when you're happy (I'm OOO for 3 weeks!)
Overview
Close #1475
Currently Danger cannot post an inline comment when it is installed with Octokit >= 8.
It is because that Octokit has changed the method signature of
create_pull_request_comment()
(octokit/octokit.rb@918af86).This PR fixes this issue by changing arguments depends on Octokit version.
Test
I added 2 specs to confirm that Danger changes its behavior depends on Octokit version.
And I also confirmed that this PR fixes the issue in other repository.
manicmaniac/danger-issue-1475#6
Bug / Known issues
The branch name of this PR is wrong. The change of method signature was introduced in Octokit v8, not v7.