RFR: 1288: Adjust the implementation of the 'reviewers' command to match the documentation [v2]

[Guoxiong Li]

On Mon, 28 Mar 2022 20:32:52 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix comments, names and copyrights.
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/ReviewersCommand.java line 136:
> 
>> 134:         var totalRequired = updatedLimits.values().stream().mapToInt(Integer::intValue).sum();
>> 135:         reply.print("The total number of required reviews for this PR, " +
>> 136:                 "which is combined by the configuration and the command, is now set to " + totalRequired);
> 
> I would suggest:
> 
> "The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to"

Fixed

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/ReviewersTracker.java line 66:
> 
>> 64:                     // Set the number for the lower roles to remove.
>> 65:                     remainingRemovals = remainingAdditional - updatedLimits.get(r);
>> 66:                     // The new number of the reviewers should larger or equal than the requirement of the '.jcheck/conf' file.
> 
> I would suggest moving this comment to before line 63 and express it something like this:
> 
> // The new value cannot the lower than the value in .jcheck/conf

Fixed

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/ReviewersTracker.java line 86:
> 
>> 84:                 var removed = Math.max(0, originalVal - remainingRemovals);
>> 85:                 updatedLimits.replace(r, removed);
>> 86:                 remainingRemovals -= (originalVal - updatedLimits.get(r));
> 
> Maybe use the local variable `removed` instead of updatedLimits.get(r). I think that reads better.

Fixed

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

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