#3986 Rename of yarp package to clarify functionality

dotnet / aspire

An opinionated, cloud ready stack for building observable, production ready, distributed applications in .NET

https://learn.microsoft.com/dotnet/aspire

MIT License

2.99k stars 283 forks source link

#3986 Rename of yarp package to clarify functionality #3992

Closed thompson-tomo closed 2 weeks ago

thompson-tomo commented 2 weeks ago

Move yarp to be under service resolution naming rather than service discovery as it is not actually using yarp for service discovery.

closes #3986

Microsoft Reviewers: Open in CodeFlow

danmoseley commented 2 weeks ago

@ReubenBond thoughts? This is breaking, right?

thompson-tomo commented 2 weeks ago

yes @danmoseley it would be breaking due to namespace & package name change but prior to GA is the best time to do this.

Key thing for me is yarp is not actually being used for ServiceDiscovery with this package.

davidfowl commented 2 weeks ago

Yes, its unlikely we'll take this PR as this code already went through API review.

danmoseley commented 2 weeks ago

Cc @mitchdenny

Yeah GA branch is now ship stoppers only pretty much. I think this just came too late.

thompson-tomo commented 2 weeks ago

If it is too late perhaps we split this in 2:

Rename package as 1 PR for v8 GA branch but leave namespace
In v9 we update the namespace to match the package

This would make the change easier for users if we don't want to change both package and namespace for v8 GA (my preference).

davidfowl commented 2 weeks ago

It's too late and I don't know why we'd make this change at all. This seems like an opinion and there's been no discussion at all (or none that I'm aware of).

thompson-tomo commented 2 weeks ago

Let me phrase it another way, when a user installs a service discovery package the clear expectation is that the provider named ie YARP is the source where services are discovered from. This is how the DNS package works. The yarp package implements a different interface to the DNS package and is actually simply resolving the urls for the destinations specified in the yarp config.

davidfowl commented 2 weeks ago

That doesn’t change what I said. There was an API review process that was followed. Thats the process you go through again with your valid feedback and the committee decide. This PR and the associated issues doesn’t follow any of the process.

danmoseley commented 2 weeks ago

I'm closing this PR for discussion to occur in the issue.