RicoSuter / NSwag

The Swagger/OpenAPI toolchain for .NET, ASP.NET Core and TypeScript.
http://NSwag.org
MIT License
6.72k stars 1.29k forks source link

Allow setting of the client interface access modifier based on the model access modifier #4820

Open pbolduc opened 6 months ago

pbolduc commented 6 months ago

This PR allows changing the client interface access modifier. This PR is to address #1363.

Changes:

This PR does not address valid combination of setting the generated C# class and model to public, but the interface internal. This would require additional work to expose additional arguments to the command line tool and changes to the UI tools. Someone would need to justify the use case that make this additional combination valid and the effort to implement.

Valid in the table means it is valid C# and will compile. This PR really addresses the last item in the table.

valid class model interface Handled?
Yes public public public Yes - worked this way before
Yes public public internal No - use case?
No public internal public No - not valid C#
No public internal internal No - not valid C#
Yes internal public public Yes - worked this way before
Yes internal public internal No - edge case?
No internal internal public No - not valid C#
Yes internal internal internal Yes - This PR fixes this specific combination
pbolduc commented 6 months ago

The failing unit tests pass locally. Is this something that I need to investigate or could it be related to the test runner setup?

image

Numpsy commented 6 months ago

The failing unit tests pass locally. Is this something that I need to investigate or could it be related to the test runner setup?

Looks like the same failure I got in #4785 - I wondered if the test timeout I mentioned at https://github.com/RicoSuter/NSwag/pull/4785#issuecomment-1998612818 simply isn't long enough for the macOS CI runner, though I don't have a Mac to test it locally with so I'm just guessing

jmevel commented 6 months ago

Awesome, thanks for your work @pbolduc !

pbolduc commented 6 months ago

Are there any issues with this PR? Our team is prevented using the MS Build task to auto generate our C# client because our models are internal. I have to keep reminding my team, they have to manually generate client using the UI and after generating the C# client, they need to edit the generated interface from public to internal.

Please advise if you would like changes by providing feedback on this PR. I have tried to ensure this PR does not introduce any regressions and only makes minimal changes to allow types to be generated as internal to the assembly.

jmevel commented 2 months ago

MacOS build failures should be fixed by #4896

https://github.com/RicoSuter/NSwag/issues/1363#issuecomment-2163715770

Now that #4896 has been merged, @pbolduc could you please re-run the Macos build? Latest run is from March 17th.

Hoping this PR could be merged soon.

Thanks

pbolduc commented 2 months ago

MacOS build failures should be fixed by #4896

#1363 (comment)

Now that #4896 has been merged, @pbolduc could you please re-run the Macos build? Latest run is from March 17th.

Hoping this PR could be merged soon.

Thanks

I am not a maintainer on this Repo. I am not clear on how I would "re-run the Macos build". Usually a maintainer needs to re-run the github actions.

jmevel commented 2 months ago

MacOS build failures should be fixed by #4896

#1363 (comment) Now that #4896 has been merged, @pbolduc could you please re-run the Macos build? Latest run is from March 17th. Hoping this PR could be merged soon. Thanks

I am not a maintainer on this Repo. I am not clear on how I would "re-run the Macos build". Usually a maintainer needs to re-run the github actions.

Ok thanks for your answer. I thought maybe the creator of the PR could manually re-run the build without having to push a new commit.

I guess we just have to wait for a maintainer then.