Skip to content

[Java] adds snapshotVersion CLI option and uses API version as artifactVersion by default #2033

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

Merged

Conversation

pablolazaro
Copy link
Contributor

@pablolazaro pablolazaro commented Jan 31, 2019

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: master, 3.4.x,4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

@bbdouglas
@JFCote
@sreeshas
@jfiala
@lukoyanov
@cbornet
@jeff9finger

Description of the PR

This PR contains two simple new features:

  • If no artifactVersion is provided in additional properties, version from API specification is used. If none of them is provided then fallbacks to default version (1.0.0)
  • Adds a snapshotVersion CLI option to build artifact version as SNAPSHOT ({version}+"-SNAPSHOT")

@pablolazaro pablolazaro changed the title [Java]: adds snapshotVersion CLI option and uses API version as artifactVersion by default [Java] adds snapshotVersion CLI option and uses API version as artifactVersion by default Jan 31, 2019
@wing328 wing328 changed the base branch from 4.0.x to master February 7, 2019 10:47
@wing328
Copy link
Member

wing328 commented Feb 7, 2019

I've retargeted the PR for master instead of 4.0.x branch.

Can you please resolve the merge conflicts when you've time? Let me know if you need any help.

@pablolazaro
Copy link
Contributor Author

pablolazaro commented Feb 7, 2019

Update: let me check everything in home because some test are broken now.

Hi @wing328, thank you for your attention.

I've resolved the merge conflicts, please let me know if anything is wrong.

Best regards.

@pablolazaro
Copy link
Contributor Author

I fixed the tests but now CircleCI fails and says "Please run 'bin/utils/ensure-up-to-date' locally and commit changes". I've done this and have pushed the changes but it still failing.

Do I should revert these last changes?

@wing328
Copy link
Member

wing328 commented Feb 9, 2019

@pablolazaro I've updated the samples via 84b4f5d. Let's see how it goes.

@wing328
Copy link
Member

wing328 commented Feb 9, 2019

@pablolazaro thanks again for the PR. Have you considered adding "-SNAPSHOT" to the artifactVersion directly instead of introducing another option?

@pablolazaro
Copy link
Contributor Author

Hey @wing328, thank you for your time, I'm seeing that all check have been passed.

It's an interesting question, let me try to explain why I considered to have separate options for version and snapshot version.

Usually in your API definitions you don't have "snapshot" versions. You would probably have a closed released version in your master branches and a potential-release version (the next version of master branch) in your development branches. In order to publish an artifact generated from development branches it's nice to mark it as "snapshot", therefore, you can use the "snapshot" option in your CI when you're generating an artifact from these kind of branches to create the POM with a SNAPSHOT version. When you're releasing a new version you can skip the option to use the API version as artifact version.

Let me know if it has sense.

@wing328
Copy link
Member

wing328 commented Feb 18, 2019

Thanks for the explanation. Let's go with this one.

Eventually, we may want to sunset some options in the Java-related generators and use additionalProperties or customized templates instead but that not likely going to happen until 5.x/6.x release.

@wing328 wing328 merged commit ad8aa7d into OpenAPITools:master Feb 18, 2019
@pablolazaro
Copy link
Contributor Author

Thanks for the merge. I hope to keep contributing as much as a I can :)

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
…ctVersion by default (OpenAPITools#2033)

* [Java]: adds snapshotVersion CLI option and uses API version as artifactVersion by default

* fix some typos

* fix naming diff between branches

* ensure-up-to-date

* update samples
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