Hi Brian,

thank you for your review.

regarding '@Deprecated(since=“9”, forRemoval=true)', since the only users of 
testlibrary classes are our own tests and we do not need to inform anyone or 
wait for a new release to remove them, I don't think it makes much sense to 
have such attributes.

PS sorry, by some reason the email unnoticeably stuck in my outbox, and I have 
already pushed the patch. if you have any further comments, I will address them 
by a separate fix.

Thanks,
-- Igor

> On May 30, 2017, at 9:36 AM, Brian Burkhalter <brian.burkhal...@oracle.com> 
> wrote:
> 
> Hi Igor,
> 
> On May 26, 2017, at 11:30 PM, Igor Ignatyev <igor.ignat...@oracle.com 
> <mailto:igor.ignat...@oracle.com>> wrote:
> 
>> this changeset introduces jdk.test.lib.RandomFactory in the top level 
>> testlibrary and updates all but java/time/ tests to use it instead of 
>> jdk.testlibrary.RandomFactory from the jdk testlibrary.
> 
> Code changes look OK provided all tests still run properly. I have not been 
> following the test package refactoring however so no comment on that.
> 
>> due to CODETOOLS-7901987[1], java/time/ tests can't use the top level 
>> testlibrary, so jdk.testlibrary.RandomFactory hasn't been removed, however 
>> it has been marked as deprecated and will be removed by JDK-8181118[2] as 
>> soon as there is promoted jtreg w/ CODETOOLS-7901987 fixed.
> 
> Should this be using “@Deprecated(since=“9”, forRemoval=true)” or something 
> like that?
> 
> Thanks,
> 
> Brian

Reply via email to