Skip to content

Conversation

wing328
Copy link
Contributor

@wing328 wing328 commented Sep 13, 2016

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 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

  • fix the location of FodyWeavers.xml
  • minor template change for better code format

Tested with

java -jar modules/swagger-codegen-cli/target/swagger-codegen-cli.jar generate \
   -i http://petstore.swagger.io/v2/swagger.json \
   -l csharp \
   -o /var/tmp/csharp/generatePropertyChanged/ --additional-properties=generatePropertyChanged=true

@wing328 wing328 force-pushed the weaver_location_fix branch from e0d922f to d1de0d7 Compare September 13, 2016 08:56
@wing328 wing328 merged commit 19047c2 into swagger-api:master Sep 13, 2016
@wing328 wing328 deleted the weaver_location_fix branch September 13, 2016 10:20
@jimschubert
Copy link
Contributor

@wing328 how are you building this? The XML is supposed to be in the project root, so I'm not sure what's being fixed here.

@wing328
Copy link
Contributor Author

wing328 commented Sep 13, 2016

I did another test to confirm it's working for me (Mac, Xamarin 5.x).

The PR aims to address this particular issue: #3547 (comment)

Does it work for you if you use the latest master to generate C# API client with generatePropertyChanged switched on?

@jimschubert
Copy link
Contributor

@wing328 thanks. I'll check this when I get a chance. My new baby was born on September 2, so it's been hectic at home.

Did you update Project.mustache? I'm on mobile which sometimes doesn't show all affected files.

@wing328
Copy link
Contributor Author

wing328 commented Sep 13, 2016

Congratulations to your new born 👏 👏 👏

I didn't update project.mustachte, which is referring to the xml file in the project root directory (not the solution root directory).

I'll ask others to give it a try as well so don't worry about it.

(I'll also setup CI for this)

@wing328
Copy link
Contributor Author

wing328 commented Sep 14, 2016

Updated CI with #3796 to cover C# API client with "PropertyChanged"

@wing328 wing328 modified the milestones: v2.2.1, v2.2.2 Sep 14, 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