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

Microsoft.Extensions.DependencyInjection.Specification.Tests is missing API documentation #87709

Closed
ViktorHofer opened this issue Jun 16, 2023 · 6 comments · Fixed by #105502
Closed
Labels
area-Extensions-DependencyInjection documentation Documentation bug or enhancement, does not impact product or test code good first issue Issue should be easy to implement, good for first-time contributors in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@ViktorHofer
Copy link
Member

See #84917 for more context.

The compiler generated XML file for Microsoft.Extensions.DependencyInjection.Specification.Tests is shipping to customers via the nuget package and is missing public API documentation. The above linked PR disables the compiler error CS1591 until all the API is documented.

When working on this, remove the NoWarn=1591 setting from the project file.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 16, 2023
@ghost
Copy link

ghost commented Jun 16, 2023

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

Issue Details

See #84917 for more context.

The compiler generated XML file for Microsoft.Extensions.DependencyInjection.Specification.Tests is shipping to customers via the nuget package and is missing public API documentation. The above linked PR disables the compiler error CS1591 until all the API is documented.

When working on this, remove the NoWarn=1591 setting from the project file.

Author: ViktorHofer
Assignees: -
Labels:

area-Extensions-DependencyInjection

Milestone: -

@steveharter steveharter added this to the 8.0.0 milestone Jun 28, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jun 28, 2023
@steveharter steveharter added documentation Documentation bug or enhancement, does not impact product or test code untriaged New issue has not been triaged by the area owner labels Jun 28, 2023
@buyaa-n buyaa-n removed the untriaged New issue has not been triaged by the area owner label Jun 28, 2023
@steveharter
Copy link
Member

Is this worth it? Documenting the test types and individual tests? We could make the test methods non-public to avoid documenting each test.

@ViktorHofer
Copy link
Member Author

It's probably not a priority. I don't know if we can make the test methods non-public. I thought those are the "public API" that other DI libraries test against.

@steveharter
Copy link
Member

I believe a [Fact] test method can be private but the class must be public.

These are the tests that other DI implementations can use to test against, however the tests are self-describing and unless there is a real benefit to creating the doc and maintaining it, I suggest closing this issue.

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Aug 17, 2023

That works for me.

Before closing though, consider updating this comment here:

@ericstj ericstj modified the milestones: 8.0.0, 9.0.0 Sep 9, 2023
@steveharter steveharter added the good first issue Issue should be easy to implement, good for first-time contributors label Jul 25, 2024
@steveharter steveharter modified the milestones: 9.0.0, 10.0.0 Jul 25, 2024
@steveharter
Copy link
Member

Just the comment needs to be updated - something like "Tests do not need to be documented."

@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Jul 25, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-DependencyInjection documentation Documentation bug or enhancement, does not impact product or test code good first issue Issue should be easy to implement, good for first-time contributors in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants