Skip to content

Conversation

@arnested
Copy link
Contributor

@arnested arnested commented Mar 4, 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

Follow-up on the improved Docker stuff in #2945 this pull request adds building and uploading a Swagger Codegen CLI Docker image to Docker Hub on releases.

@arnested
Copy link
Contributor Author

arnested commented Mar 4, 2017

I had to add running mvn verify -Psamples in batch mode (mvn --batch-mode verify -Psamples ) to make mvn be less verbose when downloading ressources (it was by Travis' log limit of 4 MB).

@arnested
Copy link
Contributor Author

arnested commented Mar 4, 2017

@fehguy, I guess you are the one to have a look at this? Would you also consider manually pushing v2.2.2 og Swagger Codegen CLI to https://hub.docker.com/r/swaggerapi/swagger-codegen-cli/?

@wing328
Copy link
Contributor

wing328 commented Mar 6, 2017

@arnested one question if you don't mind. Does it mean pull request (not yet approved) will also update the docker images and push those to the docker hub?

@wing328 wing328 added the Docker label Mar 6, 2017
@wing328 wing328 added this to the v2.2.3 milestone Mar 6, 2017
@arnested
Copy link
Contributor Author

arnested commented Mar 6, 2017

It does the same as it does for the https://hub.docker.com/r/swaggerapi/swagger-generator image.

Pushing a versioned tag for future releases and pushes to latest for master).

@wing328
Copy link
Contributor

wing328 commented Mar 6, 2017

Just to be clear. I'm not saying your PR introduces additional issue.

Since the commands are triggered by PR (not yet approved) as well, I was wondering what if the PR has issue (e.g. due to bad changes). Would the images in docker hub still be updated regardless? If yes, then maybe we should only push master to docker hub if possible (assuming my understanding of the potential issue is correct).

@arnested
Copy link
Contributor Author

arnested commented Mar 6, 2017

Nitpicking is fine 😄

Only tags will trigger a tagged push to Docker Hub.

The push to the latest on Docker Hub will happen just like the existing Swagger Generator image: For pushes to master and pull request coming from the same repos (which I don't think ever happens).

Actually it will happen whenever the DOCKER_HUB_EMAIL, DOCKER_HUB_USERNAME, and DOCKER_HUB_PASSWORD are present in the build and since the Travis configuration seems to have marked them as secure this only happens on pushes/pull requests from the same repos.

I guess it would make sense to tighten this to only pushes to master (although this is no worse than what is already done for the generator images).

I'll give it shot on improving this later today.

@arnested
Copy link
Contributor Author

arnested commented Mar 6, 2017

I have added a commit that should ensure pushes to Docker Hub only happens for master and tags.

@wing328
Copy link
Contributor

wing328 commented Mar 6, 2017

@arnested I'm sorry I sound like nitpicking 😞 and thanks for updating the PR to address my concern. The change looks good to me.

@fehguy does the change look good to you as well?

@arnested
Copy link
Contributor Author

arnested commented Mar 6, 2017

Don't worry. Nitpicking was meant in a positive way (carring for details).

@wing328 wing328 merged commit 0b9aaad into swagger-api:master Mar 12, 2017
@wing328
Copy link
Contributor

wing328 commented Mar 12, 2017

@arnested PR merged into master. Developers using the docker images will likely find this enhancement useful.

cc @fehguy

@arnested arnested deleted the feature/swagger-codegen-cli-on-docker-hub branch March 12, 2017 10:06
@arnested
Copy link
Contributor Author

Thank you, @wing328!

The image was build on master and pushed to latest as it was supposed to: https://hub.docker.com/r/swaggerapi/swagger-codegen-cli/tags/

spr3nk3ls pushed a commit to spr3nk3ls/swagger-codegen that referenced this pull request Mar 28, 2017
…-api#4912)

* Run mvn in batch-mode to be less verbose on download

* Build and upload Swagger Codegen CLI Docker image on release

* Ensure we only push master and tags to Docker Hub
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