On Tue, 15 Nov 2022 19:19:57 GMT, Maurizio Cimadamore <[email protected]>
wrote:
>> Andrew Haley has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix failing serviceability tests
>
> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java
> line 613:
>
>> 611: * @return the value of the scoped value if bound, otherwise {@code
>> other}
>> 612: */
>> 613: public T orElse(T other) {
>
> From an API perspective, wouldn't return `Optional` a more consistent choice?
> `Optional` has all methods we need to deal with this kind of stuff...
This comment is on orElse but I suspect you are suggesting that get() be
changed to return Optional. I think we'll need to get more feedback/usage of
this API before re-visiting that.
> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java
> line 229:
>
>> 227: * <li> Inheritance of {@linkplain ScopedValue scoped values} across
>> threads.
>> 228: * <li> Confinement checks. The phrase "threads contained in the task
>> scope" in method
>> 229: * descriptions means threads started in the task scope or descendant
>> scopes.
>
> sadly, the term "descendant scopes" is not defined elsewhere in this javadoc
Good poin,t, we had more setup in previous iterations.
@theRealAph I'll adjust this in the loom repo as I can't do it here.
-------------
PR: https://git.openjdk.org/jdk/pull/10952
