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

[Guoxiong Li]

> Hi all,
> 
> The `reviewers` command [documentation](https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/reviewers) states:
> 
>> Description
>> Require that at least N users with given role (defaults to Author) review the pull request before it can be integrated. The requirements are in addition to the ones specified by the .jcheck/conf file.
> 
> But the implementation doesn't match the documentation now, especially the bug reported at [JDK-PULL-6752](https://github.com/openjdk/jdk/pull/6752) ([SKARA-1288](https://bugs.openjdk.java.net/browse/SKARA-1288) also recorded this bug).
> 
> This patch fixes the bug and revises the code to match the documentation.
> 
> The logic has small change(Please see the following code). In the past, the reviewer number of the role will be the sum of the `updatedLimits.get(r)` and `remainingAdditional`. But I don't know it meets our expectation. Because the higher roles can reduce the `remainingAdditional`, I think the remaining number should be only set to `updatedLimits` but not added to `updatedLimits`. This matches the documentation `Require that at least N users with given role (defaults to Author) review the pull request`, especially the key work `at least`.
> 
> 
> -                updatedLimits.replace(r, updatedLimits.get(r) + remainingAdditional);
> +                if (remainingAdditional > updatedLimits.get(r)) {
> +                    // Set the number for the lower roles to remove.
> +                    remainingRemovals = remainingAdditional - updatedLimits.get(r);
> +                   // The new number of the reviewers should larger or equal than the requirement of the '.jcheck/conf' file.
> +                    // Because the '.jcheck/conf' file means the minimal reviewer requirement.
> +                   updatedLimits.replace(r, remainingAdditional);
> +              }
> 
> 
> The other code changes are listed below:
> - add negative number check.
> - remove the updated limits check. Because it is not needed after adding the negative number check.
> - adjust the reply message, always output the concrete reviewers. Because the merged logic of jcheck config and user `reviewers` command is a little hard, we should let the user know the result directly instead of guessing the result.
> - `decrease lower roles` in the method `ReviewersTracker#updatedRoleLimits` doesn't work as expected. I fix it.
> - add more test cases.
> 
> Thanks for taking the time to review.
> 
> Best Regards,
> -- Guoxiong

Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:

  Fix comments, names and copyrights.

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

Changes:
  - all: https://git.openjdk.java.net/skara/pull/1262/files
  - new: https://git.openjdk.java.net/skara/pull/1262/files/3880e32c..50e062f7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=skara&pr=1262&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=skara&pr=1262&range=00-01

  Stats: 37 lines in 3 files changed: 5 ins; 14 del; 18 mod
  Patch: https://git.openjdk.java.net/skara/pull/1262.diff
  Fetch: git fetch https://git.openjdk.java.net/skara pull/1262/head:pull/1262

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