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

Transient "disposable" service language updates #31738

Merged
merged 8 commits into from
Feb 8, 2024
Merged

Conversation

guardrex
Copy link
Collaborator

@guardrex guardrex commented Feb 8, 2024

Fixes #31726

Ok ... now we're cook'in with GAS! 🔥

Thanks @hakenr (and the HAVIT team) for sticking with this to the end 🥇. I included the link to the PU issue, and that makes sense given that it was punted back to .NET 10 for consideration.

Mackinnon: I'm assuming here that non-disposable transient services registered client-side are disposed correctly when the component is disposed, as they would be in the server-side case per Steve's remarks. Other than that, I think this closely matches what he said ... I stole 🦹 quite a bit of his language for these updates 👮🚓🚨 .


Internal previews

📄 File 🔗 Preview link
aspnetcore/blazor/fundamentals/dependency-injection.md ASP.NET Core Blazor dependency injection

@guardrex guardrex self-assigned this Feb 8, 2024
Copy link
Contributor

@hakenr hakenr left a comment

Choose a reason for hiding this comment

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

Consider my suggestions for an even clearer meaning.

aspnetcore/blazor/fundamentals/dependency-injection.md Outdated Show resolved Hide resolved
aspnetcore/blazor/fundamentals/dependency-injection.md Outdated Show resolved Hide resolved
Copy link
Contributor

@hakenr hakenr left a comment

Choose a reason for hiding this comment

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

lgtm 🎉

Let's try these updates ... and I'll take your "for" over "instead" suggestion now that I'm dropping the mention of disposable transient services from that sentence.
@guardrex
Copy link
Collaborator Author

guardrex commented Feb 8, 2024

Hammered 🔨 that point home a bit harder with the latest updates, and I took your "for" over "instead" suggestion because I dropped the disposable transient language from that sentence in the third paragraph.

Copy link
Member

@MackinnonBuck MackinnonBuck 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 to me, just a few comments

aspnetcore/blazor/fundamentals/dependency-injection.md Outdated Show resolved Hide resolved
aspnetcore/blazor/fundamentals/dependency-injection.md Outdated Show resolved Hide resolved
aspnetcore/blazor/fundamentals/dependency-injection.md Outdated Show resolved Hide resolved
@guardrex
Copy link
Collaborator Author

guardrex commented Feb 8, 2024

Thanks @MackinnonBuck and @hakenr. 🎉

@guardrex guardrex merged commit 10c72c7 into main Feb 8, 2024
3 checks passed
@guardrex guardrex deleted the guardrex-patch-3 branch February 8, 2024 23:24
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.

Transient "disposable" services language
3 participants