On Thu, 18 Jul 2024 18:16:48 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> Proposing a new overload method for `Process#waitFor()` which takes a
>> `Duration` for the timeout value. This will reduce the possibility for
>> making mistakes with the `TimeUnit` in the other overload method. A
>> corresponding CSR has also been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Addressing review comments
src/java.base/share/classes/java/lang/Process.java line 462:
> 460: throws InterruptedException {
> 461: Objects.requireNonNull(unit, "unit"); // throw NPE before other
> conditions
> 462:
Seems out of scope for this enhancement. Might be better in the PR to update
@implSpec as minor updates.
src/java.base/share/classes/java/lang/Process.java line 504:
> 502: return false;
> 503:
> 504: return waitForNanos(TimeUnit.NANOSECONDS.convert(duration));
I'd rather see one of these methods call the other, to make it easy keep the
behavior in sync.
The Duration can be extracted to nanos and call the existing method.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20220#discussion_r1683348791
PR Review Comment: https://git.openjdk.org/jdk/pull/20220#discussion_r1683352257
