RFR: 282: Make the review requirements more explicit in the progress list [v3]

Guoxiong Li gli at openjdk.java.net
Fri Apr 29 15:04:13 UTC 2022


On Fri, 29 Apr 2022 13:26:45 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add null check
>
> bots/pr/src/test/java/org/openjdk/skara/bots/pr/ReviewersTests.java line 46:
> 
>> 44: 
>> 45:     private static final String reviewProgressTemplate = "Change must be properly reviewed (%d reviews required, with at least %s)";
>> 46:     private static final String zeroReviewProgress = "Change must be properly reviewed (no reviews required)";
> 
> Please use uppercase underscore separated identifiers for constants.
> 
> Suggestion:
> 
>     private static final String REVIEW_PROGRESS_TEMPLATE = "Change must be properly reviewed (%d reviews required, with at least %s)";
>     private static final String ZERO_REVIEW_PROGRESS = "Change must be properly reviewed (no reviews required)";

Fixed.

> I would probably have put the logic for generating this string right here in the getter. 

Fixed. Please note the field `reviewRequirements` is not `final` now.

> I realize this gets a lot of testing through the ReviewersTest above, but it would make sense to have cheap and quick tests of just this functionality here in the jcheck package.

Added tests.

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

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


More information about the skara-dev mailing list