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] Client nuspec #4576

Merged
merged 2 commits into from
Jan 17, 2017
Merged

[csharp] Client nuspec #4576

merged 2 commits into from
Jan 17, 2017

Conversation

jimschubert
Copy link
Contributor

PR checklist

  • Read the contribution guildelines.
  • Ran the shell/batch 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)
  • Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

Per #3350, this generates a nuspec for C# client generated code.

To test, run:

./bin/csharp-petstore.sh

On Windows:

cd samples\client\petstore\csharp\SwaggerClient\src\IO.Swagger
nuget pack -Build -OutputDirectory out IO.Swagger.csproj

This will create the .nupkg file under a new directory called out.

This implementation bundles the README.md and docs.

@wing328
Copy link
Contributor

wing328 commented Jan 17, 2017

@jimschubert thanks for the PR but the CI reported the following issue:

$ /bin/bash ./bin/utils/detect_carriage_return.sh
modules/swagger-codegen/src/main/resources/csharp/nuspec.mustache
Templates contain carriage return '/r'. Please remove it and try again.

I believe the NuGET packing will still work after removing the carriage return from nuspec.mustache.

Please take a look when you've time.

@jimschubert
Copy link
Contributor Author

@wing328 I can save with line feed only. The file looks terrible in Windows without carriage returns. Is that alright? I don't understand the need to warn for carriage returns, does it cause problems somehow?

@wing328
Copy link
Contributor

wing328 commented Jan 17, 2017

@jimschubert another way is to change your git setting so that git will clean it up for you during commit: https://github.com/swagger-api/swagger-codegen/blob/master/CONTRIBUTING.md#tips

We did receive complains about auto-generated files containing '/r' before.

Personally I think it would be a good idea to standardize the line break in the project unless there's a specific use case that we must use '/r'. I'm definitely open to discussion to allow '/r' in the code or template files.

@jimschubert
Copy link
Contributor Author

@wing328 I think files predominately used by Windows users like the C# stuff should probably allow carriage returns. It might make sense to disallow it in the templates, but output with carriage returns as an option?

@wing328
Copy link
Contributor

wing328 commented Jan 17, 2017

@jimschubert one thing I also consider is the experience programming in Mac/Linux (using Mono) with the C# client/server generated by Swagger Codegen. I think it depends on what IDE/editor is being used and some editors may not have the (default) setting to hide/convert "\r\n" properly.

@wing328 wing328 merged commit 4c7d133 into swagger-api:master Jan 17, 2017
@jimschubert jimschubert deleted the csharp/3350 branch January 18, 2017 01:40
davidgri pushed a commit to davidgri/swagger-codegen that referenced this pull request May 11, 2017
* [csharp] Client nuspec

* [csharp] remove carriage returns from nuspec
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