Deprecate Observable, add ChangeNotifier, setup travis #11
Conversation
| - dev | ||
| - stable | ||
|
|
||
| script: ./tool/presubmit.sh |
| @@ -1,3 +1,36 @@ | |||
| ## 0.17.0 | |||
|
|
|||
| This is a larger change with a goal of no runtime changes for current | |||
There was a problem hiding this comment.
[nit] IMO the Changelog should be more succinct and these details should stay in the commit message
There was a problem hiding this comment.
Only reason I made this verbose is because of the implications.
| /// May use [notifyChange] to queue a change record; they are asynchronously | ||
| /// delivered at the end of the VM turn. | ||
| /// | ||
| /// [ChangeNotifier] may be extended, mixed in, or used as a delegate. |
There was a problem hiding this comment.
👍 for documenting that the class is meant to be extended
| return (_changes ??= new StreamController<List<C>>.broadcast( | ||
| sync: true, | ||
| onListen: observed, | ||
| onCancel: unobserved, |
There was a problem hiding this comment.
[optional] If you omit the trailing comma line 31 might format nicer
| /// May override to be notified when [changes] is first observed. | ||
| @override | ||
| @protected | ||
| @mustCallSuper |
There was a problem hiding this comment.
Are we mainly marking this because we assume that transitively some subclasses may need it? For future proofing?
There was a problem hiding this comment.
Yes. I'm making it more clear now.
| } | ||
| } | ||
|
|
||
| // Will be removed when Observable removes `notifyPropertyChange`. |
There was a problem hiding this comment.
should we mark it as deprecated?
| @override | ||
| /*=T*/ notifyPropertyChange/*<T>*/( | ||
| Symbol field, | ||
| /*=T*/ |
There was a problem hiding this comment.
I don't see dartfmt forcing this on its own line - did you choose it?
There was a problem hiding this comment.
I just ran dartfmt no manual formatting.
Remove stable branch, as `collection` dependency won't resolve on it.
|
LGTM |
Partial work toward #10.
0.17.0
This is a larger change with a goal of no runtime changes for current
customers, but in the future
Observablewill become a verylightweight interface, i.e.:
ObservableinterfaceChangeNotifiershould be used as a base class for these methods:Observable.observedObservable.unobservedObservable.hasObserversObservable.deliverChangesObservable.notifyChangePropertyChangeNotifiershould be used for these methods:Observable.notifyPropertyChangeObservableusesChangeNotifierimplements Observableshouldmove to implementing or extending
ChangeNotifier. In afuture release
Observablewill reduce API surface down toan abstract
Stream<List<C>> get changes.ChangeNotifierandPropertyChangeNotifierclassesObservablein a generic mannerObservable<C extends ChangeRecord>C,notifyPropertyChangeis illegal