On Thu, 9 Jan 2025 15:23:18 GMT, Alan Bateman <[email protected]> wrote:
>> Viktor Klang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Addresses PR review feedback
>
> src/java.base/share/classes/java/util/stream/Gatherers.java line 392:
>
>> 390: while (proceed
>> 391: && (current = wip.peekFirst()) != null
>> 392: && (current.isDone() || atLeastN > 0)) {
>
> It might be better to indent these two lines so that it's clearer what the
> while expression is vs. the code in the block.
Fair point—I reformatted that while clause to something which is hopefully
easier to read.
> src/java.base/share/classes/java/util/stream/Gatherers.java line 421:
>
>> 419: if (!success && !wip.isEmpty()) {
>> 420: // First signal cancellation for all tasks in
>> progress
>> 421: for(var task : wip)
>
> Minor formating nit is that you probably want a space in "for(", there are a
> few more of these in the patch.
Fixed!
> test/jdk/java/util/stream/GatherersMapConcurrentTest.java line 322:
>
>> 320: @ParameterizedTest
>> 321: @MethodSource("concurrencyConfigurations")
>> 322: public void
>> behavesAsExpectedWhenCallerIsInterrupted(ConcurrencyConfig cc) {
>
> It might be helpful for future maintainers to put a comment on the
> behaveAsExpectedXXX tests so that it's easier to figure out what they are
> testing.
That's fair, I decided to improve the names of the methods in the follow-on
commit I just pushed 👍
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22999#discussion_r1909049038
PR Review Comment: https://git.openjdk.org/jdk/pull/22999#discussion_r1909048037
PR Review Comment: https://git.openjdk.org/jdk/pull/22999#discussion_r1909047753
