Skip to content

Conversation

@Y-Sindo
Copy link
Member

@Y-Sindo Y-Sindo commented Apr 17, 2025

No description provided.

@Y-Sindo Y-Sindo requested a review from Copilot April 18, 2025 03:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds a new optional property, MaxPageSize, to the GroupMemberQueryMessage and updates its serialization, deserialization, equality checking, and test coverage.

  • Added MaxPageSize to the GroupMemberQueryMessage model with updated XML documentation.
  • Updated serialization/deserialization routines in ServiceProtocol.cs to handle the new property.
  • Updated tests in ServiceProtocolFacts.cs and the equality comparer in ServiceMessageEqualityComparer.cs to verify the new field.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/Microsoft.Azure.SignalR.Protocols.Tests/ServiceProtocolFacts.cs Added new test data entries to validate the updated binary format with MaxPageSize.
test/Microsoft.Azure.SignalR.Protocols.Tests/ServiceMessageEqualityComparer.cs Included MaxPageSize in equality checks.
src/Microsoft.Azure.SignalR.Protocols/ServiceProtocol.cs Updated serialization and deserialization logic for GroupMemberQueryMessage to include MaxPageSize.
src/Microsoft.Azure.SignalR.Protocols/ServiceMessage.cs Added the MaxPageSize property with XML comments.

vwxyzh
vwxyzh previously approved these changes Apr 29, 2025
@vwxyzh
Copy link
Contributor

vwxyzh commented Apr 29, 2025

please fix compile error.

@Y-Sindo Y-Sindo enabled auto-merge (squash) April 29, 2025 07:59
@Y-Sindo Y-Sindo closed this Apr 29, 2025
auto-merge was automatically disabled April 29, 2025 08:07

Pull request was closed

@Y-Sindo Y-Sindo reopened this Apr 29, 2025
@Y-Sindo Y-Sindo enabled auto-merge (squash) April 29, 2025 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants