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