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

[aspnet] Initial implementation of ASP.NET 5 server #2024

Merged

Conversation

jimschubert
Copy link
Contributor

See Issue #1469

Things to work on:

Gaps:

  • Missing swagger definition functionality:
    • defaultResponse
    • examples
    • auth
    • consumes
    • produces
    • nickname
    • externalDocs
    • imports
    • security
    • schema
  • Resolve allParams/hasMore issue with headerParams
  • Create functional test project stub
  • Resolve all issues with value type return values

In this commit:

  • Initial cross-platform ASP.NET 5 API server
  • Hook up swagger gen via Swashbuckle and xml comment
  • Build script (*nix) in project root
  • Dockerfile for container deployment

Things to work on:

Gaps:

* Missing swagger definition functionality:
  - defaultResponse
  - examples
  - auth
  - consumes
  - produces
  - nickname
  - externalDocs
  - imports
  - security
  - schema
* Resolve allParams/hasMore issue with headerParams
* Create functional test project stub
* Resolve all issues with value type return values

In this commit:

* Initial cross-platform ASP.NET 5 API server
* Hook up swagger gen via Swashbuckle and xml comment
* Build script (*nix) in project root
* Dockerfile for container deployment
@wing328
Copy link
Contributor

wing328 commented Feb 3, 2016

@jimschubert thank you 🍻

I'll review and let you know if I've any questions.

@jimschubert
Copy link
Contributor Author

@wing328 Thanks. It generates a pretty complete server. Nothing snazzy like services or DI. Just pushed a second commit to fix a bad merge of the Haskell Servant example in README.md when I rebased.

I did have to add a hack for operation paths. Something like /pet/{petId}?testing_byte_array=true is an invalid identifier for RouteAttribute. I just replaced the ? with / so the user can build and modify as needed.

/// <summary>
/// {{description}}
/// </summary>
public class {{classname}} : {{#parent}}{{{parent}}}, {{/parent}} IEquatable<{{classname}}>
Copy link
Contributor

Choose a reason for hiding this comment

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

[OPTIONAL] we just added partial to the model class. You might want to do the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add that. After we recently regenerated the C# client at work, I was thinking about how nice it would be if this was had the partial keyword.

I'll look at the client code, but are there any plans or best practices for partial methods? Or just the class for now?

Copy link
Contributor

Choose a reason for hiding this comment

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

To me, adding partial to model classes is just a starting point. If there're demands/use cases for adding partial to methods or other classes, we'll definitely consider it.

@wing328
Copy link
Contributor

wing328 commented Feb 3, 2016

[OPTIONAL] you may also want add \bin\windows\aspnet5-petstore-server.bat as there may be more developers using it on the Windows platform. (ref: https://github.com/swagger-api/swagger-codegen/blob/master/bin/windows/csharp-petstore.bat)

@jimschubert
Copy link
Contributor Author

@wing328 I made those changes. Let me know if there's anything else you find.

@wing328
Copy link
Contributor

wing328 commented Feb 4, 2016

Looks good to me.

wing328 added a commit that referenced this pull request Feb 4, 2016
[aspnet] Initial implementation of ASP.NET 5 server
@wing328 wing328 merged commit 2de3e2e into swagger-api:master Feb 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants