Skip to content

Conversation

inidona
Copy link
Contributor

@inidona inidona commented Apr 5, 2018

#5542 not working for us

InterfaceOnly=true for not generating pom.xml not works in every situation.

PR checklist

  • Read the contribution guidelines.
  • Ran the shell 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). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

(details of the change, additional tests that have been done, reference to the issue for tracking, etc)
Issue 5542, i added the additionalParameter "generateMeta=[true|false]" for controling generation of pom.xml and README.md. The generation of pom.xml depending on parameter interfaceOnly did not work for our situation. I generated with -DgenerateMeta=true and false

false
[main] INFO io.swagger.codegen.AbstractGenerator - writing file ...XXXResponse.java
[main] INFO io.swagger.codegen.AbstractGenerator - writing file ...XXXApi.java
[main] INFO io.swagger.codegen.AbstractGenerator - writing file ...swagger/.swagger-codegen/VERSION

and true
[main] INFO io.swagger.codegen.AbstractGenerator - writing file ...XXXResponse.java
[main] INFO io.swagger.codegen.AbstractGenerator - writing file ...XXXApi.java
[main] INFO io.swagger.codegen.AbstractGenerator - writing file ...swagger/pom.xml
[main] INFO io.swagger.codegen.AbstractGenerator - writing file ...swagger/README.md
[main] INFO io.swagger.codegen.AbstractGenerator - writing file ...swagger/.swagger-codegen/VERSION

@jeff9finger could you review this PR ?
@bbdouglas (2017/07) @JFCote (2017/08) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09)

Andreas Müller added 2 commits April 5, 2018 12:14
…rue for not generating pom.xml not works in every situation.
…rue for not generating pom.xml not works in every situation.
@inidona inidona changed the title Issue 5542, generate pom.xml with separate parameter. InterfaceOnly=t… Issue 5542, spring generate pom.xml with separate parameter. Apr 5, 2018
@wing328
Copy link
Contributor

wing328 commented Apr 6, 2018

What about using .swagger-codegen-ignore to skip files that you don't need?

@inidona
Copy link
Contributor Author

inidona commented Apr 6, 2018

I need this files. In actual master branch this files were not generated with additionalParameter interfaceOnly=true, because of this i added the new parameter generateMeta and when this is true the pom.xml and README.md will be generated. It is now possible to control with a parameter to generate pom.xml, README.md or not, and not implicit with interfaceOnly=true|false

@wing328
Copy link
Contributor

wing328 commented Apr 8, 2018

Instead of introducing another option, what about including those files when interfaceOnly=true? (users can use .swagger-codegen-ignore to skip those during code generation)

We want to avoid too many options to customize the output.

@inidona
Copy link
Contributor Author

inidona commented Apr 8, 2018

this is ok for me, so i have to revert the part of PR https://github.com/swagger-api/swagger-codegen/pull/5545/files at the location where the files were generated ?
So .swagger-codegen-ignore was the better option for the original poster ?

@inidona
Copy link
Contributor Author

inidona commented Apr 10, 2018

@wing328 can you tell me what i should do now ?

@wing328
Copy link
Contributor

wing328 commented Apr 10, 2018

@inidona If I understand correctly, you will need to change the behavior of the InterfaceOnly=true option to also generate the pom file.

@inidona
Copy link
Contributor Author

inidona commented Apr 10, 2018

@wing328 yes, i would generate the pom.xml always.

@wing328
Copy link
Contributor

wing328 commented Apr 12, 2018

@inidona sorry that I wasn't clear. My suggest is to avoid adding another option "GENERATE_META" and rather update the existing "INTERFACE_ONLY" option to generate the pom.xml as well.

Please reach me via the email address in my Github profile if you can to dicuss this further (email is much easier to draw my attention than a discussion in the issue tracker)

@inidona
Copy link
Contributor Author

inidona commented Apr 13, 2018

@wing328 i made my changes

@inidona inidona changed the title Issue 5542, spring generate pom.xml with separate parameter. Issue 5542, always generate pom and readme Apr 13, 2018
@inidona
Copy link
Contributor Author

inidona commented Apr 16, 2018

@jeff9finger could you review this PR ?
@bbdouglas (2017/07) @JFCote (2017/08) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09)

Copy link

@jeff9finger jeff9finger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems to always include pom.xml and README.md. There is no condition for interfaceOnly == true.

If I understand the conversation correctly, these files are only supposed to be included when interfaceOnly == true

@inidona
Copy link
Contributor Author

inidona commented Apr 16, 2018

Hi, no i talked with wing328 and we have consent that the files should always been generated. If a user don't won't these files he have to use .swagger-codegen-ignore

thanks
andreas

Copy link

@jeff9finger jeff9finger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Thanks for the clarification.

LGTM 👍

@wing328 wing328 merged commit 77fd35f into swagger-api:master Apr 17, 2018
@wing328 wing328 added this to the v2.4.0 milestone Apr 17, 2018
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