On Tue, 18 Aug 2020 19:50:55 GMT, Leon Linhart <github.com+4029915+themrmilchm...@openjdk.org> wrote:
> Hi, this PR fixes > [JDK-8251946](https://bugs.openjdk.java.net/browse/JDK-8251946) computing > whether the list was > actually modified instead of just returning `true`. The list was modified if > 1. it was not empty (modified by calling > `#clear()`), or if 2. it was modified as result of the `#addAll()` call. > > If you want any test coverage for this please let me know. > > I reported the issue a couple of days ago via web formula and waited for the > confirmation to open this PR but now I see > that @kevinrushforth (sorry for pinging) is already assigned to the JBS > issue. I'm not too familiar with how you handle > JBS issues or more specifically whether assigning is used to indicate that > the issue is in the assignee's "domain", or > that the assignee is already working on it. In the latter case, please feel > free to close this PR. (My OCA submission > is still pending anyway.) Change seems good, suggested minor changes. modules/javafx.base/src/main/java/javafx/collections/ModifiableObservableListBase.java line 32: > 30: import java.util.List; > 31: import java.util.ListIterator; > 32: import java.util.Objects; This import seems unused, please remove. modules/javafx.base/src/main/java/javafx/collections/ModifiableObservableListBase.java line 97: > 95: clear(); > 96: addAll(col); > 97: return true; I think following code would be more suitable here, boolean res = super.addAll(c); return res; This code is already used in two `addAll()` methods of this class. ------------- Changes requested by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/284
