Skip to content

Conversation

@Anonycoders
Copy link
Contributor

Description

  • Add refresh() method to fetch complete comment data from the API
  • Document the GitHub API limitation affecting line-related fields

Problem

When fetching review comments through review.listReviewComments(), the getLine() method always returns -1. This is because the GitHub API endpoint
/repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}/comments does not return line, original_line, side, and related fields.

Solution

  1. Added refresh() method - Implements Refreshable interface, allowing users to fetch complete comment data:
comment.refresh();
int line = comment.getLine(); // Now returns actual value
  1. Added documentation - Javadoc on getLine(), getSide(), and related methods explaining when -1/UNKNOWN is returned.
  2. Recommended workaround - Use pullRequest.listReviewComments() instead, which returns complete data.

Before submitting a PR:

  • Changes must not break binary backwards compatibility. If you are unclear on how to make the change you think is needed while maintaining backward compatibility, CONTRIBUTING.md for details.
  • Add JavaDocs and other comments explaining the behavior.
  • When adding or updating methods that fetch entities, add @link JavaDoc entries to the relevant documentation on https://docs.github.com/en/rest .
  • Add tests that cover any added or changed code. This generally requires capturing snapshot test data. See CONTRIBUTING.md for details.
  • Run mvn -D enable-ci clean install site "-Dsurefire.argLine=--add-opens java.base/java.net=ALL-UNNAMED" locally. If this command doesn't succeed, your change will not pass CI.
  • Push your changes to a branch other than main. You will create your PR from that branch.

When creating a PR:

  • Fill in the "Description" above with clear summary of the changes. This includes:
    • If this PR fixes one or more issues, include "Fixes #" lines for each issue.
    • Provide links to relevant documentation on https://docs.github.com/en/rest where possible. If not including links, explain why not.
  • All lines of new code should be covered by tests as reported by code coverage. Any lines that are not covered must have PR comments explaining why they cannot be covered. For example, "Reaching this particular exception is hard and is not a particular common scenario."
  • Enable "Allow edits from maintainers".

@codecov
Copy link

codecov bot commented Jan 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.02%. Comparing base (36f0922) to head (f1983f3).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2188   +/-   ##
=========================================
  Coverage     85.01%   85.02%           
- Complexity     2495     2496    +1     
=========================================
  Files           237      237           
  Lines          7355     7357    +2     
  Branches        388      388           
=========================================
+ Hits           6253     6255    +2     
  Misses          871      871           
  Partials        231      231           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bitwiseman
Copy link
Member

bitwiseman commented Feb 10, 2026

@Anonycoders Great catch here!

The comments on the methods and the test showing working behavior are good.

However, your comments also make it clear this is a behavior is truly bizarre and likely to cause confusion downstream (even with your comments).

I'd like you to consider a couple other options:

Option 1: Return classes that match the records

  • Add a new class GHPullRequestReview#CommentListItem that contains only the fields present on the records returned by GHPullRequestReview#listReviewComments()
  • Add a method to GHPullRequestReview#CommentListItem named pubilc GHPullRequestReviewComment readPullRequestReviewComment() that fetches the full GHPullRequestReviewComment
  • Change GHPullRequestReview#listReviewComments() return PagedIterable<GHPullRequestReview#CommentListItem>
    ** This would make it instantly clear that the endpoint is returning limited information
    ** This is a breaking change, but 2.0 is still not final. We'd need to do an added step or two to merge the change.
  • Keep your comments with some modification, people will still need them

Option 2: Auto-refresh incomplete records, hiding the bad behavior

  • add a field (internal) bool incomplete; to GHPullRequestReviewComment.
  • add a method private void refreshIncomplete() { if (incomplete) { refresh(); incomplete = false; }
  • add call to refreshIncomplete() to getters for all fields not returned as part of GHPullRequestReview#listReviewComments()
    ** Pro: Non-breaking change and quietly works around confusing behavior
    ** Con: Adds stealth REST API calls, allows users to implement potentially expensive/suboptimal behavior without being aware of the expense
  • Keep your comments with some modification, people will still need them

I think I'd prefer Option 1, it's clear and makes expensive behavior visible.

Here's the GHPullRequestReview#CommentListItem records:
https://github.com/Anonycoders/github-api/blob/f1983f3b51849ad707d5c35ad12b3b9719df91e7/src/test/resources/org/kohsuke/github/GHPullRequestTest/wiremock/pullRequestReviews/__files/9-r_h_g_pulls_482_reviews_2121304234_comments.json

And here's the full GHPullRequestReviewComment:
https://github.com/Anonycoders/github-api/blob/f1983f3b51849ad707d5c35ad12b3b9719df91e7/src/test/resources/org/kohsuke/github/GHPullRequestTest/wiremock/pullRequestReviews/__files/13-r_h_g_pulls_comments_1641771497.json

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.

GHPullRequestReviewComment.getLine() always returns -1

2 participants