Re: RFR: 8251946: ObservableList.setAll does not conform to specification [v4]

2020-10-07 Thread Ambarish Rapte
On Sat, 26 Sep 2020 16:04:16 GMT, Leon Linhart 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 c

Re: RFR: 8251946: ObservableList.setAll does not conform to specification [v4]

2020-09-30 Thread Kevin Rushforth
On Sat, 26 Sep 2020 16:04:16 GMT, Leon Linhart 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 c

Re: RFR: 8251946: ObservableList.setAll does not conform to specification [v3]

2020-09-26 Thread Leon Linhart
On Sat, 26 Sep 2020 13:51:10 GMT, Kevin Rushforth wrote: >> Leon Linhart has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Reverted incorrect change and improved test coverage > > modules/javafx.base/src/test/java/test/javafx/collections/O

Re: RFR: 8251946: ObservableList.setAll does not conform to specification [v4]

2020-09-26 Thread Leon Linhart
> 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 th

Re: RFR: 8251946: ObservableList.setAll does not conform to specification [v3]

2020-09-26 Thread Kevin Rushforth
On Fri, 25 Sep 2020 16:47:37 GMT, Leon Linhart 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 c

Re: RFR: 8251946: ObservableList.setAll does not conform to specification [v3]

2020-09-25 Thread Leon Linhart
> 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 th

Re: RFR: 8251946: ObservableList.setAll does not conform to specification [v3]

2020-09-25 Thread Leon Linhart
On Fri, 25 Sep 2020 15:49:12 GMT, Kevin Rushforth wrote: >> Makes sense to me. I changed it accordingly. > > I don't think this change is correct. `setAll(Collection)` should return > true if the list is modified. As discussed in > an [earlier comment](#issuecomment-684117392) this means return

Re: RFR: 8251946: ObservableList.setAll does not conform to specification [v2]

2020-09-25 Thread Kevin Rushforth
On Fri, 25 Sep 2020 10:23:54 GMT, Leon Linhart wrote: >> 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 m

Re: RFR: 8251946: ObservableList.setAll does not conform to specification [v2]

2020-09-25 Thread Leon Linhart
On Thu, 24 Sep 2020 18:23:54 GMT, Ambarish Rapte wrote: >> Leon Linhart has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removed unused import and addressed review comments > > modules/javafx.base/src/main/java/javafx/collections/Modifiab

Re: RFR: 8251946: ObservableList.setAll does not conform to specification [v2]

2020-09-25 Thread Leon Linhart
> 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 th

Re: RFR: 8251946: ObservableList.setAll does not conform to specification

2020-09-24 Thread Ambarish Rapte
On Tue, 18 Aug 2020 19:50:55 GMT, Leon Linhart 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 callin

Re: RFR: 8251946: ObservableList.setAll does not conform to specification

2020-09-04 Thread Kevin Rushforth
On Tue, 18 Aug 2020 19:50:55 GMT, Leon Linhart 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 callin

Re: RFR: 8251946: ObservableList.setAll does not conform to specification

2020-09-04 Thread Leon Linhart
On Thu, 20 Aug 2020 12:54:27 GMT, Kevin Rushforth 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

Re: RFR: 8251946: ObservableList.setAll does not conform to specification

2020-09-04 Thread Dalibor Topic
On Tue, 1 Sep 2020 00:23:38 GMT, Kevin Rushforth wrote: >> While adding unit tests, I noticed that I missed an important edge-case that >> has to be considered when computing if a >> list was modified. The initial implementation assumed that >>> the list was modified if >>>1. it was not empty (m

Re: RFR: 8251946: ObservableList.setAll does not conform to specification

2020-09-04 Thread Dalibor Topic
On Tue, 1 Sep 2020 15:17:18 GMT, Leon Linhart wrote: >> One overall comment while we are waiting for your OCA to be approved. >> >> I don't think the complexity of this proposed fix to `setAll` is warranted. >> I would prefer a simpler fix that returns >> `false` if both the current list and t

Re: RFR: 8251946: ObservableList.setAll does not conform to specification

2020-09-04 Thread Leon Linhart
On Tue, 1 Sep 2020 00:23:38 GMT, Kevin Rushforth wrote: > I don't think the complexity of this proposed fix to `setAll` is warranted. I > would prefer a simpler fix that returns > `false` if both the current list and the new list are empty, and `true` > otherwise. @kevinrushforth Thanks for yo

RFR: 8251946: ObservableList.setAll does not conform to specification

2020-09-04 Thread Leon Linhart
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()

Re: RFR: 8251946: ObservableList.setAll does not conform to specification

2020-09-04 Thread Kevin Rushforth
On Fri, 21 Aug 2020 14:27:15 GMT, Leon Linhart wrote: >> @TheMrMilchmann I am not actively working on that bug, so you can proceed >> with this PR. I will review it when it is >> ready. >> Once you have submitted the OCA, please add a comment to this PR with the >> `/signed` command (and nothi