Use 'line' instead of 'position'

[tick-taku]

Resolves #1565

---

Behavior

Before the change?

• The deprecated 'position' is being used in create_pull_request_comment.(https://docs.github.com/en/rest/pulls/comments?apiVersion=2022-11-28#create-a-comment)

After the change?

• Use 'line' instead of 'position'.

Other information

• I changed to specify parameters required for multiline comments, such as 'start_line', in the 'options'.

---

Additional info

Pull request checklist

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

• Yes (Please add the Type: Breaking change label)
• No

If Yes, what's the impact:

• N/A

Pull request type

Please add the corresponding label for change this PR introduces:

• Bugfix: Type: Bug
• Feature/model/API additions: Type: Feature
• Updates to docs or samples: Type: Documentation
• Dependencies/code cleanup: Type: Maintenance

---

[kfcampbell]

I've approved the Actions runs; thank you for submitting! I'm slightly torn about this: I think this is a breaking change since the signature of create_pull_request_comment is changing. Would it be better to provide an override or mark the current method as deprecated first and then remove it entirely in a new major version after?

[tick-taku]

Well...You're right! It may not be possible to provide an override because we're just changing the parameter name, and the type remains the same. So I think we have no choice but to release it in the next major version.
In terms of just using it, you can still use it as is since the type remains the same, and it's not using keyword arguments.
What do you think would be a good course of action?

[kfcampbell]

Hmm...I'm not loving any of these options, really. Perhaps the best thing to do is note in comments/docs that "position" will go away in favor of "line" in the next major version, and save this PR up for when we cut a new major version.

[tick-taku]

I see, understood! Thanks!
We will put it on hold until the next major version. I will submit a Pull Request with the additional comments.

[halorgium]

Would you consider adding an interim method which uses the new API request format?
In theory, the interim method could become an alias in the next major version.

Continuing to use the position parameter seems to result in the side parameter to behave incorrectly (every comment associates with the left-hand side).

By adopting the line parameter, the functionality behaves as expected.

My workaround is to call directly into the API:

octokit.post(
  "#{Octokit::Repository.path("user/repo")}/pulls/#{pull_id}/comments",
  {
    body: body,
    commit_id: commit_id,
    path: path,
    line: line,
    side: 'RIGHT'
  }
)

[kfcampbell]

When we merge #1596, we can/should also take this PR before cutting a breaking change.

[kfcampbell]

I think the deprecation notice has been baking long enough and we can release this breaking change now!