Skip to content

Conversation

@erikwj
Copy link
Contributor

@erikwj erikwj commented Nov 5, 2017

@jimschubert: An addition to #6818
Added scalafix for cleanup
fixed an issue with the path parameters (String :: String vs string :: string)
Some refactoring of the code
No tests yet.

@jimschubert
Copy link
Contributor

Thanks. I'll check this out in the next day or two. I know the Finch generator is a little bit of a beast as FP requires jumping through more hoops than OOP in mustache templates. Really appreciate the work!

@wing328
Copy link
Contributor

wing328 commented Nov 6, 2017

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/swagger-api/swagger-codegen/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/swagger-api/swagger-codegen/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

@erikwj
Copy link
Contributor Author

erikwj commented Nov 6, 2017 via email

@erikwj erikwj force-pushed the finch_updates branch 2 times, most recently from 22f0de7 to ff325fd Compare November 6, 2017 13:37
@erikwj
Copy link
Contributor Author

erikwj commented Nov 6, 2017

@wing328 a full rewrite as explained in post, didn't work out. So merged the commits and added author manually. Let me know if you have any comments

@wing328 wing328 merged commit 1b90a05 into swagger-api:master Nov 27, 2017
@wing328
Copy link
Contributor

wing328 commented Nov 27, 2017

Upgrade Note

date-time is mapped to "ZonedDateTime" instead of "LocalDateTime".

To roll back the change, please use the --type-mappings option.

@jimschubert
Copy link
Contributor

@erikwj @wing328 hey, I just saw the upgrade note from earlier today. I had planned on reviewing the new PR and never saw the update regarding the merge & manual author update. Sorry about that.

fwiw, the change looks good and I had no additional comments.

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