Skip to content

[C#] Asynchronous support #809

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

Merged
merged 14 commits into from
Jun 5, 2015
Merged

Conversation

Hadou1
Copy link

@Hadou1 Hadou1 commented May 29, 2015

  • Extended api.mustache for CSharp to support asynchronous calls aswell. Every generated method is now generated with a async version

@wing328
Copy link
Contributor

wing328 commented Jun 3, 2015

One minor feedback is to add XML documentation to interface methods

ref: https://msdn.microsoft.com/en-us/library/vstudio/z04awywx%28v=vs.100%29.aspx

@fehguy
Copy link
Contributor

fehguy commented Jun 5, 2015

This is great--however I'm not sure we should just replace the existing c# library with an async one. How about a new csharp-async template set?

@wing328
Copy link
Contributor

wing328 commented Jun 5, 2015

@fehguy from what I understand, the PR will add an additional method (while keeping the old/existing one). For example, a method named "GetPetByIdAsync" will be added by this change while the GetPetById method will still be available.

Last time I tested with the non-async methods (GetPetById, UploadFile, UpdatePetWithStatus, etc) and they're working fine.

@Hadou1
Copy link
Author

Hadou1 commented Jun 5, 2015

Hi guys,

what i have done is to add a interface for the API classes and as @wing328 wrote an additional method wich is implemented Async. I would definitely integrate this in the existent templates, hence this is normal pattern for me.

The reason of adding interfaces is to provide tha ability to work with dependency injection frameworks, eg. Autofac or Ninject

@fehguy
Copy link
Contributor

fehguy commented Jun 5, 2015

got it @Hadou1 @wing328. We had some template drift, but if you can help merge the logic back I will merge the PR.

@Hadou1
Copy link
Author

Hadou1 commented Jun 5, 2015

OK... will do

@Hadou1
Copy link
Author

Hadou1 commented Jun 5, 2015

@fehguy can you please tell me how to proceed in order to complete this. I'm new to this and i'm not sure how to get the changes the best way. I forked the repo and i would now like to get those latest changes. Do i have to fork again or can i somehow pull the latest changes into my repo?

@wing328
Copy link
Contributor

wing328 commented Jun 5, 2015

@Hadou1 here is what I would recommend:

@Hadou1
Copy link
Author

Hadou1 commented Jun 5, 2015

@wing328 Thanks for your reply. I also found that command... but when i try executing this i always get exception like this:

$ git pull upstream
fatal: 'upstream' does not appear to be a git repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

@wing328
Copy link
Contributor

wing328 commented Jun 5, 2015

You need to define an "upstream" (remote) first. Please refer to https://help.github.com/articles/configuring-a-remote-for-a-fork/ for more info.

@Hadou1
Copy link
Author

Hadou1 commented Jun 5, 2015

@wing328 Thanks.. got it

Fredrik Gustafsson added 2 commits June 5, 2015 12:29
@Hadou1
Copy link
Author

Hadou1 commented Jun 5, 2015

@fehguy @wing328
I now rebased the changes from develop_2.0 and fixed the conflicts. Please review/Feedback and merge...

@wing328
Copy link
Contributor

wing328 commented Jun 5, 2015

@Hadou1 Thanks for the quick turnaround

@fehguy fehguy merged commit 277287a into swagger-api:develop_2.0 Jun 5, 2015
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.

5 participants