OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.46k stars 6.49k forks source link

[csharp] massively improve csharp templates code #18885

Closed filipe-silva closed 3 weeks ago

filipe-silva commented 3 months ago

massively improve csharp templates code

improved apiclient.mustache to keep it dry, sharing a single exec with Action<> delegate. improved api.mustache to keep it dry, used chain constructors, kept RequestOptions in a single method, set configuration with alias directive moved fqn types to using directive for cleaner code removed 'this' from variables/props that are redundant fix CSharpClientDeepObjectTest code is now DRY so count must only be one

Note: This is a PR based on #16990 with resolved merge conflicts, and up to date with master

PR checklist

@mandrean @shibayan @Blackclaws @lucamazzanti @iBicha @wing328

wing328 commented 3 months ago

please PM me via Slack when you've time. thanks

devhl-labs commented 3 months ago

Your additions to ApiClient.mustache use tabs. The rest of the document and all the other mustache templates use spaces. This is a regular comment instead of a review because my browser tends to freeze looking at large diffs :(

Tabs in some other places too. I recommend you turn on whitespace in your editor. If you're using vscode I've noticed you have to toggle the whitespace option to get them to show.

wing328 commented 3 months ago

Please instead submit a small PR for each enhancement or bug fix so that it can be reviewed and merged more easily.

wing328 commented 3 months ago

and looks like some of these enhancements were already done in the latest master. e.g. https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/csharp/modelGeneric.mustache#L401

my suggestion is to file new smaller PRs based on the latest master and we can get these merged quicker to keep the ball rolling.