prontolabs / pronto

Quick automated code review of your changes
MIT License
2.62k stars 245 forks source link

Remove usage of deprecated position argument for GitHub comments #455

Closed pbstriker38 closed 10 months ago

pbstriker38 commented 11 months ago

Fixes: #453

ashkulz commented 11 months ago

@pbstriker38 I'm not sure this works with older versions of octokit -- what do you think of bumping the dependency? Without verifying that it works, I'd be a bit hesitant to merge this as-is.

pbstriker38 commented 11 months ago

@ashkulz I did verify that it works with both versions. On Octokit 7.2.0 it sends both position and line params. The API ignores postion and uses line. On Octokit 8.0.0 the params are merged into the options and thus the line is merged.

The other option I can do here is to not use Octokit#create_pull_comment method and to just use Octokit#post directly like so:

options = {
  body: comment.body,
  commit_id: pull_sha || comment.sha,
  path: comment.path,
  line: comment.position
}
client.post("#{Octokit::Repository.path(slug)}/pulls/#{pull_id}/comments", options)
pbstriker38 commented 11 months ago

I did not test earlier Octokit versions though. I can try and test those too.

ashkulz commented 10 months ago

Thanks for the contribution and being patient while we figured things out, @pbstriker38!