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

Consider making more types internal #2805

Closed
martincostello opened this issue Apr 16, 2024 · 3 comments · Fixed by #3007
Closed

Consider making more types internal #2805

martincostello opened this issue Apr 16, 2024 · 3 comments · Fixed by #3007
Assignees
Labels
version-major A change suitable for release in a major version
Milestone

Comments

@martincostello
Copy link
Collaborator

Review the public API and see if there's any types that are public that don't need to be, such as SwaggerMiddleware. That gives us more agility to change implementation details, like adding new constructor overloads (see #2801).

It may be that some types don't need to be public and are only done so through oversight or to be accessible to unit tests. In the case of tests, it might be too tricky to do with the way that some assemblies are strong named and others aren't, as [InternalsVisibleTo] requires IVT'd assemblies to also be strong named, and all dependencies of a project need to be strong named for it to be, which creates issues where a test project depends on one project that is signed and another that isn't.

Context: #2418 (comment)

Copy link
Contributor

This issue is stale because it has been open for 60 days with no activity. It will be automatically closed in 14 days if no further updates are made.

@github-actions github-actions bot added the stale Stale issues or pull requests label Jun 20, 2024
@martincostello martincostello removed the stale Stale issues or pull requests label Jun 20, 2024
@ahoke-cr
Copy link

You can also specify InternalsVisibleTo inside the .csproj file which doesn't require any funny business with assemblies:

<ItemGroup>
     <InternalsVisibleTo Include='Swashbuckle.AspNetCore.SomeTestProject' />
</ItemGroup

@martincostello
Copy link
Collaborator Author

Good point, we do that here already:

<ItemGroup>
<InternalsVisibleTo Include="Swashbuckle.AspNetCore.IntegrationTests" Key="00240000048000009400000006020000002400005253413100040000010001000b80d002f0437a94c4f6d77dfc29ce54df1040f66374cb7122ec5ddab3907a6dfe609a07ffd49b9aa6731fe3f8de4da7979ddea73e949c86effc4bec3f469cc36e788cf58c42c4be755efd1afd996ccab9f0db3455b834219f6614d00fe0410cc307c4eced320fd65fcc501f135dc537d7d09dcc1d2a572d24f8459a86469ec5" />
<InternalsVisibleTo Include="Swashbuckle.AspNetCore.SwaggerGen.Test" Key="00240000048000009400000006020000002400005253413100040000010001000b80d002f0437a94c4f6d77dfc29ce54df1040f66374cb7122ec5ddab3907a6dfe609a07ffd49b9aa6731fe3f8de4da7979ddea73e949c86effc4bec3f469cc36e788cf58c42c4be755efd1afd996ccab9f0db3455b834219f6614d00fe0410cc307c4eced320fd65fcc501f135dc537d7d09dcc1d2a572d24f8459a86469ec5" />
</ItemGroup>

@martincostello martincostello added the version-major A change suitable for release in a major version label Sep 30, 2024
@martincostello martincostello self-assigned this Sep 30, 2024
@martincostello martincostello added this to the v7.0.0 milestone Sep 30, 2024
martincostello added a commit that referenced this issue Sep 30, 2024
Make all the middleware classes internal as they have no useful public-facing functionality (e.g. virtual members).

Resolves #2805.
martincostello added a commit that referenced this issue Oct 9, 2024
Make all the middleware classes internal as they have no useful public-facing functionality (e.g. virtual members).

Resolves #2805.
martincostello added a commit that referenced this issue Oct 15, 2024
Make all the middleware classes internal as they have no useful public-facing functionality (e.g. virtual members).

Resolves #2805.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version-major A change suitable for release in a major version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants