Skip to content
This repository was archived by the owner on Jul 18, 2024. It is now read-only.

Conversation

@AnthonyAmanse
Copy link
Contributor

Replaced BLUEMIX_AUTH to TRAVIS_PULL_REQUEST

Quick change of $NODE_PORT variable to 30080

Copy link
Contributor

@mlangbehn mlangbehn left a comment

Choose a reason for hiding this comment

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

I added a few comments in line. Let's also add a more descriptive commit message.


if [[ -z "$BLUEMIX_AUTH" ]]; then
if [[ "$TRAVIS_PULL_REQUEST" != "false" ]]; then
echo -e "\033[0;33mFork detected; not authenticating to Bluemix.\033[0m"
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep in mind that this is not actually detecting a fork, it is detecting a pull request. Let's update the message to reflect this.

main(){

if [[ -z "$BLUEMIX_AUTH" ]]
if [[ "$TRAVIS_PULL_REQUEST" != "false" ]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we want to only run this test if a pull request is detected? The point of this test is so that we can have a kubernetes deployment in CI.

Changed scripts to enhance Travis CI. Various scripts change to check for TRAVIS_PULL_REQUEST instead of BLUEMIX_AUTH. Change $NODE_PORT to 30080 in test-kubernetes.sh. kubeadm test is now ran regardless if it is a PR or not.
Copy link
Contributor

@mlangbehn mlangbehn left a comment

Choose a reason for hiding this comment

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

Looks great, thanks.

Copy link
Member

@loafyloaf loafyloaf left a comment

Choose a reason for hiding this comment

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

I'm late to the party, as usual. But looks good - thanks @AnthonyAmanse

@AnthonyAmanse AnthonyAmanse merged commit 6365c1c into IBM:master Oct 25, 2017
@AnthonyAmanse AnthonyAmanse deleted the fix-scripts branch July 2, 2018 17:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

3 participants