-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
[typescript-angular] Add fileNaming
configuration property
#767
Conversation
fileNaming
configuration property
@@ -449,13 +455,13 @@ public String toApiName(String name) { | |||
} | |||
return initialCaps(name) + serviceSuffix; | |||
} | |||
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove these spaces in an empty line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, done
} | ||
return camelize(removeModelSuffixIfNecessary(name), true) + serviceFileSuffix; | ||
} | ||
return this.convertUsingFileNamingConvention(name) + serviceFileSuffix; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use spaces instead of tabs and remove trailing spaces at line 463
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace all tabs with 4-space in this file. Let me know if you need any help on that.
cc @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) |
3141ca9
to
94b9985
Compare
fileNaming
configuration propertyfileNaming
configuration property
I rebased my PR on the latest master (3.2.1-SNAPSHOT) |
If there's no further feedback/question on this PR (other than the discussion in the issue tracker), I'll merge it today |
Unfortunately and from my point of view you cannot merge this PR in this current state. We have to find a way to process 'imports' correctly before... |
@softdays my understanding is that you've tested with the |
2235051
to
ca8d962
Compare
@wing328 : ok I think everything is fine now, please have a look. |
Thanks @softdays cc @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) for another review |
4190ae4
to
d045df0
Compare
last rebase just replaces 'dash-case' to 'kebab-case' |
d045df0
to
7fcd491
Compare
7fcd491
to
3733249
Compare
@wing328 : ok I've checked out, built and run my tests, everything looks good for me |
@@ -375,7 +381,7 @@ public void postProcessParameter(CodegenParameter parameter) { | |||
List<Map<String, Object>> imports = (List<Map<String, Object>>) operations.get("imports"); | |||
for (Map<String, Object> im : imports) { | |||
im.put("filename", im.get("import")); | |||
im.put("classname", getModelnameFromModelFilename(im.get("filename").toString())); | |||
im.put("classname", im.get("classname")/*getModelnameFromModelFilename(im.get("filename").toString())*/); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggestion: remove commented code: /*getModelnameFromModelFilename(im.get("filename").toString())*/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok no problem but I'm not sure of the branch to make the change ... could you point me to the right direction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should do it in resolve_issue727
(the branch of this PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR merged into master. Thanks for your contribution.
@softdays thanks again for the PR, which is included in the v3.3.0 minor release: https://twitter.com/oas_generator/status/1046941449609068544 |
…Tools#767) * resolve OpenAPITools#727 * remove commented code
Add support for changing output file name strategy.
fileNaming
camelCase
,kebab-case
camelCase