On Fri, 28 Apr 2023 06:30:28 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
>> Nir Lisker has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains three commits:
>>
>> - Added missing space
>> - Merge branch 'master' into 8302816_Refactor_sorting-related_classes
>> - Initial commit
>
> modules/javafx.base/src/main/java/com/sun/javafx/collections/ObservableListWrapper.java
> line 45:
>
>> 43: *
>> 44: */
>> 45: public class ObservableListWrapper<E> extends
>> ModifiableObservableListBase<E> implements SortableList<E>, RandomAccess {
>
> Looks like an unintended change. Can be reverted.
The formatter is set to 120 characters in a line, I think it's fine to leave as
is.
> modules/javafx.base/src/main/java/com/sun/javafx/collections/ObservableSequentialListWrapper.java
> line 244:
>
>> 242: }
>> 243:
>> 244: private SortHelper helper;
>
> Looks like this only got moved from line# 41. Can be placed back to reduce
> diff.
I aligned it with `ObservableListWrapper` where the field is placed near where
it's used.
> modules/javafx.base/src/main/java/javafx/collections/FXCollections.java line
> 687:
>
>> 685: * @param <T> the element type of the list
>> 686: * @param list the list to sort
>> 687: * @param comparator the comparator to determine the order of the
>> list. A {@code null} value indicates that the
>
> Line looks lengthy, may be let's break into two
The max line length is 120
(https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md).
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1041#discussion_r1180289219
PR Review Comment: https://git.openjdk.org/jfx/pull/1041#discussion_r1180290732
PR Review Comment: https://git.openjdk.org/jfx/pull/1041#discussion_r1180287540
