Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[aspnetcore] Make the use of Swashbuckle optional #110

Merged
merged 2 commits into from
May 20, 2018

Conversation

FantasyTeddy
Copy link
Contributor

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

  • The sourceFolder option was not handled properly in the ASP.NET Core server generation, which is now fixed.
  • The use of Swashbuckle for documenting the server was made optional by introducing an useSwashbuckle option. (Default: true)

Technical Committee

@jimschubert
Copy link
Member

Thanks. I have a pretty full weekend, but I'll try to evaluate as soon as possible.

One question, though... ASP.NET Core, in both the 1.0 original version of the generator and after upgrading to 2.0, errors for me that code has to exist under src. Does this option work for you, or is it provided to allow users so flexibility? If you're able to compile when source folder is changed, can you share your OS and .NET Core version info so I can try?

For reference, I received the errors under OS X 10.11 (.net core 1.0), 10.13 (.net core 2.0), and Windows 10 (.net core 1.0/2.0). I'll be trying the different options in this PR, as well, I just want to ensure if I have errors that I update to the successful version.

@FantasyTeddy
Copy link
Contributor Author

I tested my sourceFolder fix with Windows 10 (.NET Core 2.0) and everything seems to run as intended. (I'm no expert though...)

I also quickly checked under macOS 10.13 (.NET Core 2.0) and I didn't encounter any problems either.

@jimschubert
Copy link
Member

jimschubert commented May 19, 2018

Cool, thanks for checking and providing version info. I'm wondering if I had some setting causing an override. I've recently upgraded my machine, so if something is different I'd unfortunately have no insight into what changed. If the option works for others in the community, obviously it's a valuable change; we've just never had verification before that changing the source folder works for others.

The reason I had to ask is that someone once opened a PR on swagger-codegen to make source folder configurable, and when I asked the same question as above, he wasn't able to build with a custom source folder.

I might be able to review this tonight. My plans for this weekend have upended and now I have more on my plate, but mostly tomorrow.

@jimschubert
Copy link
Member

Thanks. I got some time to test this out this morning and it looks great.

Tested both options on macOS High Sierra and they work as expected.

Tested with --additional-properties sourceFolder=asdf:

dotnet msbuild Org.OpenAPITools.sln
dotnet run --project asdf/Org.OpenAPITools/Org.OpenAPITools.csproj
cd asdf/Org.OpenAPITools/
dotnet run

Also, tested both with JetBrains Rider and the project built and ran successfully.

Whatever issues we had previously with changing the src folder is obviously not an issue anymore. If anyone experiences issues, my recommendation is to use the latest version of dotnet for creating a new project.

@jimschubert jimschubert merged commit d9d6530 into OpenAPITools:master May 20, 2018
@FantasyTeddy
Copy link
Contributor Author

Haha, I tested with the same folder name, so I hope we did not just get lucky 😄

@FantasyTeddy FantasyTeddy deleted the aspnetcore-improvements branch May 21, 2018 06:39
jimschubert added a commit to jimschubert/openapi-generator that referenced this pull request May 23, 2018
* master: (36 commits)
  jaxrs-cxf-cdi: fix outer enum (OpenAPITools#131)
  Move bash argv opt to end of ags line in scripts (OpenAPITools#124)
  Reduce CI logging (OpenAPITools#119)
  Download elm dependencies without prompting user. (OpenAPITools#118)
  [aspnetcore] Make the use of Swashbuckle optional (OpenAPITools#110)
  DefaultGenerator: fix NullPointerException (OpenAPITools#109)
  [Java] use html entities in javadoc of generated code (OpenAPITools#106)
  Update PULL_REQUEST_TEMPLATE.md
  [java] Enum in array of array (OpenAPITools#66)
  Rename datatype to dataType in CodegenProperty (OpenAPITools#69)
  update elm test to compile all elm files (OpenAPITools#95)
  Fix Petstore example for Elm (OpenAPITools#96)
  Update Docker documentation (OpenAPITools#97)
  CaseFormatLambda has been added, params for Rest-assured client has been refactored (OpenAPITools#91)
  Update integration.md
  [Clojure] Add util method to set the api-context globally (OpenAPITools#93)
  [JaxRS-Java] issue with implFolder on windows, and required fields generation for containers (OpenAPITools#88)
  Set parameters allowableValues dynamically (OpenAPITools#65)
  Meta: set version for "build-helper-maven-plugin" (OpenAPITools#89)
  Fix javadoc issues in "openapi-generator" module (OpenAPITools#84)
  ...
@wing328 wing328 modified the milestones: 3.0.1, 3.0.0 Jun 17, 2018
@theletterf
Copy link

Is this available in 4.0.0?

@jimschubert
Copy link
Member

@theletterf this option was introduced in our first major release, 3.0.0. If you experience issues with the option, please do open a bug.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants