Remove the old OnDisconnected overload

[halter73]

See @moozzyk's comment on the outdated diff to see why this makes sense.

[davidfowl]

Do we actually care that these are sequential? Can we do a Task.WhenAll?

[DamianEdwards]

Calling events in parallel on the same object doesn't seem right. I'd leave it sequential.

[DamianEdwards]

Any reason we're using ContinueWith with the Unwrap instead of one of our Then overloads?

[davidfowl]

Should be finally probably. Why do we care if one failed

[halter73]

Finally doesn't take a Func, and I need to return the Task from the second call to OnDisconnected. Should I make a new Finallly helper that takes a Func?

[moozzyk]

In my opinion we should remove the OnDisconnect() overload entirely for the following reasons:

• we will start breaking people who are using both overloads - they suddenly will start getting duplicate notifications. This is an implicit breaking change you would only discover at runtime (can be hard to track)
• having two overloads can create a lot of confusion
• you don't need to worry if the order invoking is important

So, because we are making a breaking change anyway removing the OnDisconnect() overload seems better since the change can be discovered at design time when upgrading to the new version and will make the API clean.
(already talked to @halter73 about this)

[halter73]

@moozzyk 👍

[DamianEdwards]

Yeah good point. It's the change we probably should have made in the first place. Simplifies everything and we don't need to explain what one overload does over the other or add any of this new task plumbing code.

[moozzyk]

🚢🇮🇹