Skip to content

Remove usage of deprecated position argument for GitHub comments#455

Merged
ashkulz merged 1 commit into
prontolabs:masterfrom
pbstriker38:fix_deprecated_pr_comment_position
Nov 9, 2023
Merged

Remove usage of deprecated position argument for GitHub comments#455
ashkulz merged 1 commit into
prontolabs:masterfrom
pbstriker38:fix_deprecated_pr_comment_position

Conversation

@pbstriker38

@pbstriker38 pbstriker38 commented Nov 3, 2023

Copy link
Copy Markdown
Contributor
  • This allows usage of Octokit v8.0.0

Fixes: #453

@pbstriker38 pbstriker38 requested a review from a team as a code owner November 3, 2023 00:19
@pbstriker38 pbstriker38 force-pushed the fix_deprecated_pr_comment_position branch from 9c61395 to 5a49d92 Compare November 3, 2023 00:21
@ashkulz

ashkulz commented Nov 3, 2023

Copy link
Copy Markdown
Member

@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

pbstriker38 commented Nov 3, 2023

Copy link
Copy Markdown
Contributor Author

@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

Copy link
Copy Markdown
Contributor Author

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

  • 4.7.0
  • 5.x.x
  • 6.x.x

Comment thread lib/pronto/github.rb
@ashkulz ashkulz merged commit 61beff7 into prontolabs:master Nov 9, 2023
@ashkulz

ashkulz commented Nov 9, 2023

Copy link
Copy Markdown
Member

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

@ashkulz

ashkulz commented Jan 11, 2025

Copy link
Copy Markdown
Member

0.11.3 has been released to RubyGems. Publishing future versions should be simpler due to implementation of Trusted Publishing workflow (c7007de) 😁

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow Octokit 8.0

2 participants