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

[CSharp] Add auto-generated CSharp documentation in Markdown #2595

Merged
merged 6 commits into from
Apr 17, 2016

Conversation

KevinGlinski
Copy link
Contributor

No description provided.

@wing328
Copy link
Contributor

wing328 commented Apr 14, 2016

@KevinGlinski thanks for the PR :)

Shall we keep the navigation links to different sections (models, list of apis, etc) to make it easier for developers to jump to different sections in the doc?

@wing328
Copy link
Contributor

wing328 commented Apr 14, 2016

@KevinGlinski also if you've time, I wonder if you can help update the README as well.

@KevinGlinski
Copy link
Contributor Author

should be good to go now

@wing328
Copy link
Contributor

wing328 commented Apr 14, 2016

@Kesh92 thanks Kevin.

I used git_push.sh to upload the C# SDK to https://github.com/wing328/petstore-csharp/blob/master/README.md#documentation-for-models

As you can see, there's a minor issue with the hyperlink text to the models. May I know if you've cycle to fix it ?

Or I can file another PR to address that after merging this one?

@jimschubert
Copy link
Contributor

@wing328 I haven't looked at the doc generation stuff yet. Is there a switch on CLI that opts-out of doc generation?

One problem I could see is if an api uses a namespace versioning strategy. For example, rather than host versions of an API separately, special routing or delegation is used per version and models are namespaced/packaged for example as models/v1/Category and models/v2/Category. I haven't tested this PR against the scenario, but it looks like Category will get flattened and documented once? I think there would be two fixes for this if they're not in place:

  1. Opt-in to the generated api documentation. I brought it up in a PR related to JavaScript documentation generation, but it's likely people would like to use another documentation tool related to their language (dox for JavaScript, Sandcastle for C#).
  2. Support more complex model namespaces.

I don't think those are blockers for this PR, though.

@wing328
Copy link
Contributor

wing328 commented Apr 17, 2016

@jimschubert currently, there's no CLI switch to ops-out the doc generation and thanks for pointing about the potential issue with complex model namespaces.

I prefer (1) to begin with so that documentations are generated by default and users can use the CLI switch to opt it out.

For (2), we can consider that as a day-2 requirement with low priority.

I'll create a task for tracking the CLI switch to opt-out the doc generation.

@wing328
Copy link
Contributor

wing328 commented Apr 17, 2016

@Kesh92 thanks again for the PR. I'l submit another PR to make some minor enhancements/bug fixes.

@KevinGlinski
Copy link
Contributor Author

@jimschubert , @wing328 i was updating from the v2.1.4 release and was caught off guard a little by the doc changes. we were using yard for ruby, jsdoc for js and doxygen for c#, python and java and we'll be moving to using the swagger doc because then it gives us a consistent format between languages on our developer site. My biggest problem was that the generation was a little too much. We already had a .gitignore and readme.md and then they were blown away by the generation, so feature toggles would be nice in situations like this.

@wing328
Copy link
Contributor

wing328 commented Apr 18, 2016

@KevinGlinski thanks for the feedback, which is similar to this one: #1956 (comment)

We plan to implement a .swagger-codegen-ignore file (similar to .gitignore) so that users can put down files that should not be overwritten as part of the code generation. Would this help to resolve the issue you're facing?

@wing328 wing328 mentioned this pull request Apr 27, 2016
17 tasks
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.

3 participants