RFR: 1145: NPE in GitHubPullRequest::reviews

[Magnus Ihse Bursie]

On Mon, 30 Aug 2021 20:28:32 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

> The core of this change is to fix an NPE in GitHubPullRequest::reviews, which happens when a commit that a review refers to no longer exists (presumably because of a rebase or forced update of the review branch). The introduction of a possible null value in Review::hash has some ripple effects however, so this gets a bit more complicated.
> 
> I've opted to wrap the return value of Review::hash and ReviewComment::hash with Optional to properly force every caller to handle the situation. I haven't seen this happen for ReviewComment, but I would be surprised if the same thing didn't apply there from the GitHub side.

Marked as reviewed by ihse (Reviewer).

forge/src/main/java/org/openjdk/skara/forge/Review.java line 62:

> 60:     /**
> 61:      * The hash for the commit for which this review was created. Can be null if the commit
> 62:      * no longer exists.

I think technically the Optional is empty, rather than "null".

forge/src/main/java/org/openjdk/skara/forge/ReviewComment.java line 55:

> 53:     /**
> 54:      * The hash for the commit for which this review comment was created. Can be null if the commit
> 55:      * no longer exists.

Same here...

-------------

PR: https://git.openjdk.java.net/skara/pull/1211