-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Release] Version 2.0.0 #26
Conversation
* editorconfig and codeformatting * rewrite * docs * throttle-event-args * another check to not reconnect while disconnecting/closing * naming * docs * changelog * reusable TaskHelper * docs * docs * downgrade from version 7 to version 5 of Microsoft.Extensions.Logging.Abstractions * downgrade from net7.0 to net5.0 * fix from integration-test * upgrade to net6 * reduced some overheaad, as mentioned/suggested by Bukk94 * ChangeLog * whispers are obsolete * ChangeLog * ChangeLog: Consideration/Proposal * fixing reconnectionpolicy-issue * fixing reconnectionpolicy-issue * ThrottlingPeriod * get MessageType-values outside while-loop the values wont change at runtime * logging * ToString() * moved and added tests * tests * usings * naming * diagnostic messages * suppress diagnostic messages * OnData has never been used * docs * todo removed, cause it got unnecessary * checked * naming * use build variable * remove * remove * remove * formatting * visibility * removed OnReconnectedEventArgs OnConnectedEventArgs is used, cause what happens is indicated by the EventHandler that is called * update version from 1.0.6 to 1.1.0 * push up ThrottlerService to TwitchClient and update version to 2.2.0 PubSub has its own PING/PONG-Timers PubSub only subscribes to Events PubSub only receives push-messages from twitch * WaitOneDuration * return-value for throttlerservice re-introduced... * changelog * assertion * remove whisper stuff * started changelog * rename method SendIRC to SpecificClientSend changed visibility from internal to protected comments updated * Send can be and is called by several methods so send has to be synchronized/locked * changelog * missed the important thing - sry * removed: event EventHandler<OnStateChangedEventArgs> OnStateChanged * removed comments locking is done in ABaseClient.Send() * give MonitorTaskAction a chance to catch cancellation * Addressed PR comments, renamed properties based on guidelines * Formatted code, updated logic for ConnectionWatchDog * Code formatting for unit tests * Added github workflow and improvements from Syzuna * Updated SSL port for TCP Client, added logging abstractions * Properly updated properties, improved naming, removed duplicate close on streams * Remove lingering option from TCP Client * Fixed code path for net6 and higher * Reworked NoReconnection policy, further code improvements * Removed broken test --------- Co-authored-by: CMR <[email protected]>
* Removed all GetAwaiter().GetResult() and replaced it with proper async/await Renamed all methods that handles tasks with Async postfix Changed IClient interface to be async Replaced lock with semaphore Fixed tests to not wait after disposing * Disable tests parallelization
code improvement and bug fix in `WebSocketClient`
enable nullable reference types
added some awaits to watchDog
improve logging performance
Async Task Events
…Exceptions, add proper IsRunning to ConnectionWatchDog
Fixed double disconnection during reconnect
Generate documentation file
/// </summary> | ||
internal CancellationToken Token => _cancellationTokenSource.Token; | ||
|
||
internal static TimeSpan TimeOutEstablishConnection => TimeSpan.FromSeconds(15); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ill assume 15 seconds is a standard practice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if its a standard practice but its a reasonable amount imo
<VersionPrefix>2.0.0</VersionPrefix> | ||
<VersionSuffix>$(VersionSuffix)</VersionSuffix> | ||
<Authors>swiftyspiffy, Prom3theu5, Syzuna, LuckyNoS7evin</Authors> | ||
<Company>swiftyspiffy, Prom3theu5, Syzuna, LuckyNoS7evin</Company> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feel like we should add some reference to Github contributors, since a decent portion of this code is now written by new-ish people :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with that.
How would you like to do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it's realistic to include every new contributor in this list, so maybe just "swiftyspiffy, Prom3theu5, Syzuna, LuckyNoS7evin, and Github contributors" or something? We'd prob need to do something similar for the other packages as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add this later though, lets not block on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I would like to add the one other PR to this before merging tho.
Did you have time to look at that one too? Bcs I didnt yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referring to #27 ? I can look tonight (in a few hours).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Co-authored-by: neon-sunset <[email protected]>
switch to File Scope Namespace, and more
OnSendFailedEventArgs: data -> message rename
Changelog: