Remove TRAVIS env vars entirely #241
Conversation
bin/build_container_image
Outdated
| @@ -3,7 +3,11 @@ | |||
| REGISTRY=${REGISTRY:-"docker.io"} | |||
| ORGANIZATION=${ORGANIZATION:-"manageiq"} | |||
| DEFAULT_TAG="latest" | |||
| [[ $TRAVIS_BRANCH != "master" ]] && DEFAULT_TAG="$DEFAULT_TAG-$TRAVIS_BRANCH" | |||
| if [[ "$GITHUB_EVENT_NAME" == "pull_request" ]]; then | |||
| DEFAULT_TAG="$DEFAULT_TAG-pr" | |||
There was a problem hiding this comment.
@bdunne Thoughts here? SInce the actual name doesn't matter, do we even care about this? I was thinking we could just leave it as "latest". That would meant this whole section can just be:
[[ "$GITHUB_EVENT_NAME" != "pull_request" -a $GITHUB_REF != "refs/heads/master" ]] && DEFAULT_TAG="$DEFAULT_TAG-$GITHUB_REF_NAME"There was a problem hiding this comment.
I noticed previously, that if run locally, this would generate latest- as the name, because TRAVIS_BRANCH was blank (actually it would probably blow up if run locally)...thinking I might fix that local use case as well
There was a problem hiding this comment.
I updated with this and fixed the latest- thing. Also fixed it so that branches with / will work (even though that doesn't effect master, it could affect pushes to forks, where the fork has the push event.
There was a problem hiding this comment.
This PR is an example where it will tag as latest: https://github.com/ManageIQ/manageiq-rpm_build/runs/4587369238?check_suite_focus=true#step:4:3079
There was a problem hiding this comment.
I'm hesitant to add GITHUB_EVENT_NAME, maybe we can get the ref from git itself?
There was a problem hiding this comment.
I can try, but we need to know if it's a PR or a push build...not sure how you get that out of git. Basically, we need push builds that are not master (i.e. morphy, lasker, etc.)
| @@ -12,7 +16,7 @@ set -e | |||
|
|
|||
| docker build . -t $IMAGE_NAME | |||
|
|
|||
| [[ $TRAVIS_PULL_REQUEST != "false" ]] && exit 0 | |||
| [[ -z "$REGISTRY_USERNAME" ]] && exit 0 | |||
There was a problem hiding this comment.
I thought it simpler to just check for the presence of the REGISTRY_USERNAME, as opposed to checking for anything CI specific
Fryguy
force-pushed
the
remove_travis
branch
2 times, most recently
from
0a63798
to
6cf4297
Compare
| @@ -3,7 +3,8 @@ | |||
| REGISTRY=${REGISTRY:-"docker.io"} | |||
| ORGANIZATION=${ORGANIZATION:-"manageiq"} | |||
| DEFAULT_TAG="latest" | |||
| [[ $TRAVIS_BRANCH != "master" ]] && DEFAULT_TAG="$DEFAULT_TAG-$TRAVIS_BRANCH" | |||
| [ "$GITHUB_EVENT_NAME" == "push" -a "$GITHUB_REF" != "refs/heads/master" ] && \ | |||
| DEFAULT_TAG="$DEFAULT_TAG-${GITHUB_REF_NAME/\//-}" | |||
There was a problem hiding this comment.
$ GITHUB_EVENT_NAME=push GITHUB_REF_NAME="master" bin/build_container_image
docker.io/manageiq/rpm_build:latest
$ GITHUB_EVENT_NAME=push GITHUB_REF_NAME="foobar" bin/build_container_image
docker.io/manageiq/rpm_build:latest-foobar
$ GITHUB_EVENT_NAME=push GITHUB_REF_NAME="feature/foobar" bin/build_container_image
docker.io/manageiq/rpm_build:latest-feature-foobar
$ bin/build_container_image
docker.io/manageiq/rpm_build:latestThere was a problem hiding this comment.
What do you think of switching to latest-masterso that we can get rid of this conditional logic?
There was a problem hiding this comment.
It's super common to have latest be the latest, so I'm hesitant to not use latest now that we've already started.
Fryguy
force-pushed
the
remove_travis
branch
from
6cf4297
to
b6760fb
Compare
|
@bdunne This is ready to go. |
|
Checked commit Fryguy@b6760fb with ruby 2.6.3, rubocop 1.13.0, haml-lint 0.35.0, and yamllint **
|
|
Backported to |
Remove TRAVIS env vars entirely (cherry picked from commit 59b85ac)
No description provided.