-
Notifications
You must be signed in to change notification settings - Fork 762
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
Fix FakeTimeProvider namespace, add readme #4544
Conversation
...ibraries/Microsoft.Extensions.Diagnostics.Probes.Tests/TcpEndpointHealthCheckServiceTests.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.TimeProvider.Testing/README.md
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.TimeProvider.Testing/README.md
Outdated
Show resolved
Hide resolved
Note that I intentionally used the Time namespace instead of TimeProvider, since having a type as a namespace component leads to all sorts of weird/unfortunate confusion with the C# compiler. |
@geeknoid ok, I can appreciate that. We should still re-consider if A) the package and namespace should align, and B) if there would be some other namespace that would be more appropriate, this is the only type in this namespace and it doesn't correlate with the type being tested/extended (System.TimeProvider). I'm not concerned about putting the type in a more common namespace because the type name is unique and the package will only be added to test projects that need it. Alternatives:
|
I've reverted the namespace change for now. |
Do you plan on tracking that in the same fit & finish workitem or want to log a new one in favor of closing the fit & fnish one? I'm fine either way, as long as we make sure we revisit this in the next couple of weeks 😃 |
Using the same work item is easier. |
The
Microsoft.Extensions.Time.Testing
namespace was mismatched to the package nameMicrosoft.Extensions.TimeProvider.Testing;
.Added a readme.
Microsoft Reviewers: Open in CodeFlow