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

8.0 Update: SignalR Config: stateful reconnect (try 2) #30850

Merged
merged 11 commits into from
Oct 31, 2023

Conversation

wadepickett
Copy link
Contributor

@wadepickett wadepickett commented Oct 26, 2023

Internal review: version 8, stateful reconnect section of SignalR Config

Internal review: What's New: SignalR: Stateful reconnect

Fixes #30674

Added new section: Configure stateful reconnect

(I had to close an earlier PR of this #30829 and start over since I introduced some merge conflicts that were a bit too messy.)


Internal previews

📄 File 🔗 Preview link
aspnetcore/release-notes/aspnetcore-8.0.md What's new in ASP.NET Core 8.0
aspnetcore/signalr/configuration.md ASP.NET Core SignalR configuration

@wadepickett wadepickett self-assigned this Oct 26, 2023
@wadepickett
Copy link
Contributor Author

Also updated per @BrennanConroy request (in previous closed version of this PR) to move the server example to the top example.
"I think this should be listed first. Server must allow stateful reconnect otherwise client settings are ignored.
We also don't need to put it for both clients, if it's at the top that should be obvious 😄"

@wadepickett
Copy link
Contributor Author

@BrennanConroy, I closed the earlier PR for this and started over with this clean one to get through a merge conflict and a messed up branch.

Your requested change to move the server hub endpoint example up is in. I also cleaned up some repeated statements to make it a little more concise.

Does this look good?

@wadepickett wadepickett marked this pull request as ready for review October 26, 2023 22:10
@wadepickett
Copy link
Contributor Author

wadepickett commented Oct 27, 2023

@BrennanConroy,

  • I added the server global and hub specific StatefulReconnectBufferSize examples.
  • Cleaned up some very minor items in the doc.
  • Updated the 8.0 What's New doc to mirror all changes and linked back to SingalR Config.

Hopefully, this all looks good. Thanks again for your help!

Internal review: version 8, stateful reconnect section of SignalR Config

Internal review: What's New: SignalR: Stateful reconnect

Copy link
Contributor

@tdykstra tdykstra left a comment

Choose a reason for hiding this comment

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

Looks good, feel free to not take the suggestions other than the typo fix.

aspnetcore/signalr/configuration.md Outdated Show resolved Hide resolved
aspnetcore/signalr/configuration.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

You already link to the Configuration article -- you could omit the duplication of configuration details here, just announce the availability of SR and explain what it is, then rely on the link to the Configuration doc for the details about how to enable it. Just a thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! A good point. I considered not calling out the configuration options themselves and linking instead, but looking over the whole What's new doc, we tend to provide an example of the API or property that is new or changed. So I decided to keep with what that topic was already doing.

@wadepickett wadepickett merged commit 126e004 into main Oct 31, 2023
2 checks passed
@wadepickett wadepickett deleted the wadepickett/30674SignalConfigStateful2 branch October 31, 2023 18:04
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.

8.0: SignalR: stateful reconnect config documentation
3 participants