On Mon, 14 Aug 2023 02:01:20 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>>> Looks good now. Do you still prefer to repeat the behavior of the >>> subscription in each of the new methods over the centralized one in [#1069 >>> (comment)](https://github.com/openjdk/jfx/pull/1069#discussion_r1261873147)? >> >> I'm really not quite sure where to put that. I don't think it would go well >> with `Subscription` since it assumes observables, and things like >> `ObservableValue#when`. We'd need to word it a lot more general then, and >> it would be hard to refer to `ObservableValue#when`, or general rules about >> garbage collection (I think that would depend on the method providing the >> subscriptions, which may not have such restrictions). >> >> Reading `ObservableValue` documentation though, I think it could use some >> updating. It mentions change and invalidation events, and their listeners, >> but nothing about the subscriptions as of yet. > >> > Looks good now. Do you still prefer to repeat the behavior of the >> > subscription in each of the new methods over the centralized one in [#1069 >> > (comment)](https://github.com/openjdk/jfx/pull/1069#discussion_r1261873147)? >> >> I'm really not quite sure where to put that. I don't think it would go well >> with `Subscription` since it assumes observables, and things like >> `ObservableValue#when`. We'd need to word it a lot more general then, and it >> would be hard to refer to `ObservableValue#when`, or general rules about >> garbage collection (I think that would depend on the method providing the >> subscriptions, which may not have such restrictions). >> >> Reading `ObservableValue` documentation though, I think it could use some >> updating. It mentions change and invalidation events, and their listeners, >> but nothing about the subscriptions as of yet. > > Yes, on `Observable` (invalidation) or `ObservableValue` (change) is the > place I was thinking of. > @nlisker Any other feedback? Not anything beyond the comment on updating `Observable/Value`'s documentation for the new methods. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1177#issuecomment-1682903160
