Removed ReactiveUI #661
Removed ReactiveUI #661
Conversation
distantcam
force-pushed
the
removing-rxui
branch
from
a498442
to
2a2f342
Compare
|
👍 @distantcam I always enjoy the smell of a dependency removal 😌 I'm afraid I need to put my 🔰 hat on and ask to be lead by the hand through these changes... |
|
@adamralph Yeah I figured. The changes are "small" in that there's only 2 things being removed (ReactiveList and ReactiveCommand) but then the replacement code is non-trivial. We can go through it next week. |
|
@distantcam This looks good to me. Feel free to merge after your session with Adam. |
| @@ -0,0 +1,23 @@ | |||
| namespace ServiceInsight.ExtensionMethods | |||
There was a problem hiding this comment.
This class no longer contains extension methods so I guess it needs to go in another namespace.
| } | ||
| } | ||
|
|
||
| public void RaiseCanExecuteChanged() |
There was a problem hiding this comment.
I thought we had.
| using System.Threading; | ||
| using System.Windows.Input; | ||
|
|
||
| class ObservableCommand : ICommand |
There was a problem hiding this comment.
Does this class need to be thread-safe WRT accessing the isExecuting and latest fields? (I noticed the use Interlocked.Exchange in Dispose.)
| Key = h.Key, | ||
| Value = h.Value | ||
| })); | ||
| KeyValues.Add(new MessageHeaderKeyValue { Key = item.Key, Value = item.Value }); |
There was a problem hiding this comment.
Does it matter that we are no longer batching these changes up into a single change notification?
distantcam
force-pushed
the
removing-rxui
branch
from
bb0872e
to
13c2285
Compare
|
@adamralph Made the changes as discussed in Lisbon. Please review. |
| CanExecuteChanged?.Invoke(this, EventArgs.Empty); | ||
| } | ||
|
|
||
| public void Dispose() |
There was a problem hiding this comment.
I just realised that this class does not implement IDisposable and this method has no callers, so it's redundant. Should we remove it, or should this class be IDisposable?
| } | ||
| } | ||
|
|
||
| public void RaiseCanExecuteChanged() |
|
@adamralph Please review |
We're not really using ReactiveUI so I've removed it.