Skip to content

Support customizing filename in M.E.ApiDescription.Server tool #56974

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Aug 12, 2024
Merged

Support customizing filename in M.E.ApiDescription.Server tool #56974

merged 6 commits into from
Aug 12, 2024

Conversation

MauriceChocoSwiss
Copy link
Contributor

Add option to specify custom OpenAPI output file name like Swashbuckle.AspNetCore.Cli

Description

Added new CLI parameter to specifiy a output file name when generating OpenAPI JSON

Fixes #56973 (in this specific format)

@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jul 24, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 24, 2024
@MauriceChocoSwiss
Copy link
Contributor Author

@dotnet-policy-service agree

@captainsafia captainsafia changed the base branch from release/9.0-preview7 to main July 24, 2024 17:12
@MauriceChocoSwiss MauriceChocoSwiss force-pushed the Lambish/GetDocumentInsiderImprove branch from 4342d4c to d85cbc0 Compare July 24, 2024 21:42
@MauriceChocoSwiss MauriceChocoSwiss force-pushed the Lambish/GetDocumentInsiderImprove branch from d85cbc0 to 5970e87 Compare July 24, 2024 21:46
@captainsafia captainsafia mentioned this pull request Jul 30, 2024
Open
captainsafia
captainsafia reviewed Jul 31, 2024
View reviewed changes
Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left another round of comments to hopefully get this to the finish line.

Mostly around renaming the argument so it's a little bit clearer that it is modifying the filename (which happens to default to the project name).

Also, can you add some tests in this file so we have coverage for this? The other tests should be a good example of what to do.

@dotnet-policy-service dotnet-policy-service bot added the area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI label Aug 4, 2024
@MauriceChocoSwiss
Copy link
Contributor Author

I've renamed the command like you recommend, add a verification on the value to use only "valid" characters and tests :) Hope this will be good this time :)

@build-analysis build-analysis bot mentioned this pull request Aug 4, 2024
Open
3 tasks
@MauriceChocoSwiss
Copy link
Contributor Author

I've seen that the Helix test failed but by looking the log output I can't find whats wrong :/ Can you tell me ?

captainsafia
captainsafia reviewed Aug 5, 2024
View reviewed changes
Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few final comments then we should be good to go.

I've seen that the Helix test failed but by looking the log output I can't find whats wrong :/ Can you tell me ?

This test failure is unrelated to your PR. We have some flakey tests in the repo that can cause these failures. Usually, we're able to quarantine or re-run the build and have things resolved.

src/Tools/dotnet-getdocument/src/ProjectOptions.cs Outdated
@@ -27,6 +27,12 @@ public void Configure(CommandLineApplication command)
Platform = command.Option("--platform <Target>", Resources.PlatformDescription);
ProjectName = command.Option("--project <Name>", Resources.ProjectDescription);
RuntimeFrameworkVersion = command.Option("--runtime <RUNTIME_IDENTIFIER>", Resources.RuntimeDescription);
command.Option("--prefix-output", Resources.PrefixDescription);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this changes? I believe the options in GetDocumentCommand is sufficient for this case.

@MauriceChocoSwiss
Copy link
Contributor Author

Hi, I'm waiting your reply to commit or not. Can you tell me if it's good for you to let this options available to have them in the -help ?

@captainsafia
Copy link
Member

Hi, I'm waiting your reply to commit or not. Can you tell me if it's good for you to let this options available to have them in the -help ?

Hmmm....can you clarify what you mean? Are you talking about my feedback here about removing the extraneous options? If so, I'd prefer to be conservative about the change we're making here and limit it to just the filename option, especially as we approach RC1.

@MauriceChocoSwiss
Copy link
Contributor Author

MauriceChocoSwiss commented Aug 12, 2024 •
edited
Loading

Hi, I'm waiting your reply to commit or not. Can you tell me if it's good for you to let this options available to have them in the -help ?

Hmmm....can you clarify what you mean? Are you talking about my feedback here about removing the extraneous options? If so, I'd prefer to be conservative about the change we're making here and limit it to just the filename option, especially as we approach RC1.

Yes it's that. I'll remove the extra options and commit as soon as possible.
I'll make another PR to add those options later because we don't see them on dotnet-getdocument with --help param even if we can use them, see picture
image

@captainsafia captainsafia changed the title Add option to specify custom OpenAPI output file name like Swashbuckle.AspNetCore.Cli Support customizing filename in M.E.ApiDescription.Server tool Aug 12, 2024
@captainsafia captainsafia merged commit abbd67b into dotnet:main Aug 12, 2024
26 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-rc1 milestone Aug 12, 2024
@MauriceChocoSwiss
Copy link
Contributor Author

🎉🎉🎉
Thank you ! And sorry for all these laborious reviews... I'll try to make future PR better !

@captainsafia
Copy link
Member

🎉🎉🎉 Thank you ! And sorry for all these laborious reviews... I'll try to make future PR better !

No problem! Thanks for taking the time to submit a PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@captainsafia captainsafia captainsafia approved these changes

Assignees

@captainsafia captainsafia

Labels
area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
9.0-rc1
Development

Successfully merging this pull request may close these issues.

Add option to specify custom OpenAPI output file name like Swashbuckle.AspNetCore.Cli
2 participants
@MauriceChocoSwiss @captainsafia