-
Notifications
You must be signed in to change notification settings - Fork 541
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
Deprecating Aspire.Hosting.Dapr #7198
Conversation
Tests are migrated to Community Toolkit
On to a better home! cc @IEvangelist |
@aaronpowell are you going to move the dapr issues? |
@@ -1,12 +0,0 @@ | |||
<Project Sdk="Microsoft.NET.Sdk"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with removing the dapr playground app, but I wonder if removing the tests is a good idea. We are still building/shipping the code, it feels like we should still run the tests for it until we remove the code completely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your call - I can revert that commit. The reason that I deleted them was to make it very clear in the code that this is unsupport.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests a pretty minimal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it probably doesn't matter much either way. Just as a pattern it feels weird to delete the tests, but keep the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like I said, your call and I can reinstate them if you'd be more comfortable.
@@ -21,6 +21,7 @@ public static class IDistributedApplicationBuilderExtensions | |||
/// <param name="builder">The distributed application builder instance.</param> | |||
/// <param name="configure">Callback to configure dapr options.</param> | |||
/// <returns>The distributed application builder instance.</returns> | |||
[Obsolete("The Dapr integration has been migrated to the Community Toolkit. Please use the CommunityToolkit.Aspire.Hosting.Dapr integration.", error: false)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my learning, does putting the Obsolete
attribute on the whole static class not work? Or is it something wth extension methods that forces us to put it on each individual method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know... I didn't try 🤣
I don't think we can directly migrate the issues since it's cross orgs on GitHub. |
Let's see what we need the follow up steps to be. @IEvangelist , @alistairmatthews we're moving dapr to the community toolkit, I know we just rewrote those docs! |
We did. There is a /docs folder in the community toolkit. Maybe they could use our updated dapr.md? |
@davidfowl is the NuGet packages changing at all? Does the community toolkit have access to the |
@IEvangelist yes it is changing CommunityToolkit/Aspire#408 |
Closes #7049
This PR deprecates the
Aspire.Hosting.Dapr
package in favor of the Community Toolkit version (PR tracking its merge is here: CommunityToolkit/Aspire#408).Removed the test project as it's not needed (code migrated to the Toolkit) and marked all members of the project with
Obsolete
(but not set as error to give people a release to upgrade).