RFR: 2331: Improve review notes [v3]

[Erik Joelsson]

On Thu, 18 Jul 2024 20:21:41 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> It's just a suggestion, since you don't like it and I am ok with your pr, I will approve it now.
>
> Usually, making a minimally necessary change is good, and parsing end-user text is bad. In fact, I'd go to great lengths to avoid "stringly typed" code that operates on strings that are also subject to change.
> 
> Here, I initially thought I might be violating some encapsulation boundary by bringing a dependency. Maybe I still am. If nothing else, it's not the first class imported from that package:
> 
>      import org.openjdk.skara.jcheck.JCheckConfiguration;
>     +import org.openjdk.skara.jcheck.TooFewReviewersIssue;

I think it's ok to return the list of jcheck issues, but the variable name `issues` can be confusing as an `Issue` in the bots is usually some form of Jira Issue. I would recommend `jcheckIssues` to avoid confusion.

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

PR Review Comment: https://git.openjdk.org/skara/pull/1679#discussion_r1712142501