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

Added support subprotocols derived from WebSocket #125

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mzapataj
Copy link

Added support subprotocols derived from WebSocket such as PrimusJs

Copy link
Owner

@Marfusios Marfusios left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, totally missed notification about this PR.
Can you please process the review comments?

/// <summary>
/// Validate if an incoming message is from sub-protocol
/// </summary>
Predicate< string> IsSubprotocolMessage { get; set; }
Copy link
Owner

Choose a reason for hiding this comment

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

Where is this used?

@@ -1,7 +1,7 @@
using System;

// ReSharper disable once CheckNamespace
namespace Websocket.Client.Models
Copy link
Owner

@Marfusios Marfusios Sep 1, 2023

Choose a reason for hiding this comment

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

This is a breaking change, put it back or we will need to deploy it under version 5.x.

@@ -41,6 +40,34 @@ public partial class WebsocketClient : IWebsocketClient
private readonly Subject<ReconnectionInfo> _reconnectionSubject = new Subject<ReconnectionInfo>();
private readonly Subject<DisconnectionInfo> _disconnectedSubject = new Subject<DisconnectionInfo>();


/// <summary>
///
Copy link
Owner

Choose a reason for hiding this comment

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

Comments please

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.

2 participants