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

Fixed docker publish #228

Merged
merged 2 commits into from
Mar 27, 2017
Merged

Fixed docker publish #228

merged 2 commits into from
Mar 27, 2017

Conversation

alex-arzner-pro
Copy link
Contributor

@alex-arzner-pro alex-arzner-pro commented Mar 23, 2017


This change is Reviewable

- Add copyright notices

- Currently, CI pushing images only with latest tag:
  https://travis-ci.org/Mirantis/k8s-AppController/jobs/203410745#L1998-L2001
  No one paid attention to this, because the "after_success" step failure
  doesn't fail build:
  travis-ci/travis-ci#758
  And at the moment a good fix is to use this as the last entry of
  "script" step instead of "after_success" step.
@dshulyak
Copy link
Contributor

:lgtm:


Review status: 0 of 3 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@jellonek
Copy link
Contributor

Really good.


Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions.


scripts/docker_publish.sh, line 40 at r1 (raw file):

    exit 1
  else
      if [ "${TRAVIS_TEST_RESULT}" -ne 0 ]; then

Lack of consistency in indenting.


scripts/docker_publish.sh, line 46 at r1 (raw file):

  fi

  if [ "${TRAVIS_PULL_REQUEST_BRANCH}" != "" ]; then

In all other tests for such case is used ! -z (better would be -n) - why there is != ""?


scripts/docker_publish.sh, line 49 at r1 (raw file):

    echo "Processing PR ${TRAVIS_PULL_REQUEST_BRANCH}"
    exit 0
  else

Same again. Lack of consistency in indenting.


scripts/import.sh, line 38 at r1 (raw file):

  fi

  for node in $(seq 1 "${NUM_NODES}"); do

👍


Comments from Reviewable

- Fixed syntax
@alex-arzner-pro
Copy link
Contributor Author

Review status: 2 of 3 files reviewed at latest revision, 4 unresolved discussions.


scripts/docker_publish.sh, line 40 at r1 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

Lack of consistency in indenting.

Done.


scripts/docker_publish.sh, line 46 at r1 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

In all other tests for such case is used ! -z (better would be -n) - why there is != ""?

Done.


scripts/docker_publish.sh, line 49 at r1 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

Same again. Lack of consistency in indenting.

Done.


Comments from Reviewable

@jellonek
Copy link
Contributor

:lgtm:


Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@pigmej
Copy link
Contributor

pigmej commented Mar 24, 2017

:lgtm: but @dshulyak pls take a look.


Reviewed 2 of 3 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@alex-arzner-pro
Copy link
Contributor Author

Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


scripts/import.sh, line 38 at r1 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

👍

thx.


Comments from Reviewable

@dshulyak
Copy link
Contributor

:lgtm:


Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


Comments from Reviewable

@dshulyak dshulyak merged commit 1523ab3 into Mirantis:master Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants