Skip to content
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

Merged
merged 28 commits into from
Sep 8, 2023
Merged

[Release] Version 2.0.0 #26

merged 28 commits into from
Sep 8, 2023

Conversation

Syzuna
Copy link
Member

@Syzuna Syzuna commented Jul 3, 2023

Changelog:

  • More or less complete rewrite of the underlying clients to make them more stable and a push for more "asyncness"
  • Throttling was removed from Communications and reimplemented in Client
  • Reworked logging
  • Added AsyncEvents

Bukk94 and others added 23 commits April 17, 2023 19:24
* 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 <github.christian-riedel@outlook.de>
* 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
…Exceptions, add proper IsRunning to ConnectionWatchDog
Fixed double disconnection during reconnect
/// </summary>
internal CancellationToken Token => _cancellationTokenSource.Token;

internal static TimeSpan TimeOutEstablishConnection => TimeSpan.FromSeconds(15);
Copy link
Member

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

Copy link
Member Author

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>
Copy link
Member

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 :)

Copy link
Member Author

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?

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member Author

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

Copy link
Member

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).

Copy link
Member

@swiftyspiffy swiftyspiffy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

OnSendFailedEventArgs: data -> message rename
@swiftyspiffy swiftyspiffy merged commit 9f1c618 into master Sep 8, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants