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.

Sign up for GitHub

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

Jump to bottom

Improve Docker Tags #390

Merged
merged 1 commit into from
Jul 5, 2018
Merged

Improve Docker Tags #390

merged 1 commit into from
Jul 5, 2018

Conversation

jmini
Copy link
Member

@jmini jmini commented Jun 25, 2018

This PR improve the way Docker-Tags are made. See the discussion in #364.

In addition to what we have now (latest and tags for version), new tags for SNAPSHOT version are introduced:

  • 3.0.x Tag: contains first 3.0.2-SNAPSHOT and after the 3.0.2 release it contains 3.0.3-SNAPSHOT and so on.
  • 3.1.x Tag: contains first 3.1.0-SNAPSHOT and after the 3.1.0 release it contains 3.1.1-SNAPSHOT and so on.
  • 4.0.x Tag: contains first 4.0.0-SNAPSHOT and after the 4.0.0 release it contains 4.0.1-SNAPSHOT and so on.

@jmini jmini mentioned this pull request Jun 25, 2018
Closed
wing328
wing328 reviewed Jul 5, 2018
View reviewed changes
.travis.yml
docker push $DOCKER_GENERATOR_IMAGE_NAME
echo "Pushed to $DOCKER_GENERATOR_IMAGE_NAME";
docker push $DOCKER_CODEGEN_CLI_IMAGE_NAME
echo "Pushed to $DOCKER_CODEGEN_CLI_IMAGE_NAME";
Copy link
Member

Choose a reason for hiding this comment

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

If docker push $DOCKER_GENERATOR_IMAGE_NAME fails but docker push $DOCKER_CODEGEN_CLI_IMAGE_NAME succeeds, is it correct to say that the CI job will still build successfully?

(I think such situation rarely occurs but still want to confirm the behavior in case that happens)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question.
Did my modification changed this?

Was this the reason why there was 2 items in the old version?

- if [ $DOCKER_HUB_USERNAME ]; ... => DOCKER_GENERATOR_IMAGE_NAME
- if [ $DOCKER_HUB_USERNAME ];  ... => DOCKER_CODEGEN_CLI_IMAGE_NAME

My idea to bring both together was to reduce copy-paste code, but I can split it back to 2 items.

Copy link
Member

Choose a reason for hiding this comment

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

Was this the reason why there was 2 items in the old version?

I wasn't the one who added it but that could be the reason.

My idea to bring both together was to reduce copy-paste code, but I can split it back to 2 items.

I definitely like your idea and formatting to make the 1-liner bash command more readable.

What about checking the exit status of line 141 before proceeding further?

Copy link
Member Author

Choose a reason for hiding this comment

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

What about checking the exit status of line 141 before proceeding further?

Good Idea, am not not really an expert in this domain.

@wing328 wing328 added this to the 3.1.0 milestone Jul 5, 2018
@jmini
Copy link
Member Author

jmini commented Jul 5, 2018

From the doc https://docs.travis-ci.com/user/customizing-the-build

Note the set -ev at the top. The -e flag causes the script to exit as soon as one command returns a non-zero exit code. This can be handy if you want whatever script you have to exit early. It also helps in complex installation scripts where one failed command wouldn’t otherwise cause the installation to fail.

The -v flag makes the shell print all lines in the script before executing them, which helps identify which steps failed.

We are using the flag:

openapi-generator/.travis.yml

Lines 92 to 94 in ed1ce37

script:
# fail fast
- set -e

So I think we the case described by @wing328 seems to be covered.

@wing328
Copy link
Member

wing328 commented Jul 5, 2018

👌

Merging this into master. Thanks for the enhancement.

@wing328 wing328 merged commit edf24d8 into master Jul 5, 2018
@wing328 wing328 added the Docker label Jul 5, 2018
@jmini jmini deleted the improve_docker_tag branch July 5, 2018 10:05
jmini added a commit that referenced this pull request Jul 6, 2018
@jmini
Revert "Improve Docker Tags (#390)"
d43801a 
This reverts commits:

* 036570d.
* edf24d8.
@jmini
Copy link
Member Author

jmini commented Jul 6, 2018

The change introduced by this PR did not work on travisCI. It was reverted with commit d43801a

jimschubert added a commit to jimschubert/openapi-generator that referenced this pull request Jul 7, 2018
@jimschubert
Merge branch 'master' into generator-validation-option
232166e 
* master: (116 commits)
  3.1.0 release (OpenAPITools#486)
  Fix broken link to openapi generator plugin README (OpenAPITools#484)
  show warning message for nodejs server only (OpenAPITools#481)
  Add grokify to Go technical committee (OpenAPITools#479)
  [Slim] Improve codebase decouple (OpenAPITools#438)
  Ensure typescript samples are up to date (OpenAPITools#444)
  Update README.md
  [Golang][client] delete sample output dir before rebuild (OpenAPITools#477)
  update petstore samples (OpenAPITools#478)
  Revert "Improve Docker Tags (OpenAPITools#390)"
  update go client test dependencies (OpenAPITools#468)
  [Golang][client] fix for schema definition name `file` (OpenAPITools#433)
  Fix '.travis' file (syntax)
  make LICENSE GitHub display compatible (OpenAPITools#467)
  Improve Docker Tags (OpenAPITools#390)
  [Golang][client] fix file suffix for _test.go (OpenAPITools#449)
  Remove copy section (OpenAPITools#463)
  Add link to presentation (OpenAPITools#465)
  Use postProcessOperationsWithModels(Map, List) (OpenAPITools#431)
  [C] Adding petstore sample written in C (OpenAPITools#306)
  ...
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
@jmini @wing328
Improve Docker Tags (OpenAPITools#390)
f8c1d6a
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
@jmini
Revert "Improve Docker Tags (OpenAPITools#390)"
850d4b5 
This reverts commits:

* 036570d.
* edf24d8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wing328 wing328 wing328 left review comments

Assignees
No one assigned
Labels
Docker Enhancement: CI/Test
Projects
None yet
Milestone
3.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jmini @wing328