RFR: 1145: NPE in GitHubPullRequest::reviews

[Erik Joelsson]

On Tue, 31 Aug 2021 10:46:03 GMT, Magnus Ihse Bursie <ihse 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.
>
> 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".

Right, in my mind I was documenting the state of the field rather than the return value of the method. I can change it.

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

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