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

Not using files from template folders with libraries inside them #4628

Merged

Conversation

eblis
Copy link
Contributor

@eblis eblis commented Jan 23, 2017

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

#4627 - When looking for template files also look in the libraries subfolder of the supplied templateFolder, before checking the embedded locations.

If the template files reside in the library subfolder in the template dir they are never checked and used, instead the embedded files (from the library subfolder) are used instead.

…ibraries subfolder of the supplied templateFolder, before checking the embedded locations.
@wing328 wing328 added this to the v2.2.2 milestone Feb 6, 2017
@wing328 wing328 modified the milestones: v2.2.2, v2.2.3 Feb 22, 2017
 - 1st the code will check if there's a <template folder>/libraries/<library> folder containing the file
 - 2nd it will check for the file in the specified <template folder> folder
 - 3rd it will check if there's an <embedded template>/libraries/<library> folder containing the file
 - 4th and last it will assume the file is in <embedded template> folder.

 Added unit test to test template dir overloading.
@eblis
Copy link
Contributor Author

eblis commented Mar 10, 2017

While working on this pull request I discovered another issue, when library folders are present.
If specifying an external template folder which uses libraries, if both the main template folder and the library folder contain the same file, the main template file will override the one in the library, but it feels like the library version of the file should win (like it does for embedded template folders).

I have modified the pull request to take this into consideration, while also adding a unit test to check that this does indeed happen.

  1. 1st the code will check if there's a <template folder>/libraries/<library> folder containing the file
  2. 2nd it will check for the file in the specified <template folder> folder
  3. 3rd it will check if there's an <embedded template>/libraries/<library> folder containing the file
  4. 4th and last it will assume the file is in <embedded template> folder.

@wing328
Copy link
Contributor

wing328 commented Mar 20, 2017

@eblis thanks for the PR with additional tests and test cases. I did some tests and the result looks good.

@wing328 wing328 merged commit 21657f8 into swagger-api:master Mar 20, 2017
spr3nk3ls pushed a commit to spr3nk3ls/swagger-codegen that referenced this pull request Mar 28, 2017
…gger-api#4628)

* swagger-api#4627 - When looking for template files also look in the libraries subfolder of the supplied templateFolder, before checking the embedded locations.

* Reworked the order in which template files are searched for:
 - 1st the code will check if there's a <template folder>/libraries/<library> folder containing the file
 - 2nd it will check for the file in the specified <template folder> folder
 - 3rd it will check if there's an <embedded template>/libraries/<library> folder containing the file
 - 4th and last it will assume the file is in <embedded template> folder.

 Added unit test to test template dir overloading.
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