-
Notifications
You must be signed in to change notification settings - Fork 58
Bitbucket Pipelines support #233
base: master
Are you sure you want to change the base?
Conversation
check-diff.sh
Outdated
@@ -69,10 +70,16 @@ function set_environment_variables { | |||
DIFF_BASE=${DIFF_BASE:-$TRAVIS_COMMIT^} | |||
fi | |||
DIFF_HEAD=${DIFF_HEAD:-$TRAVIS_COMMIT} | |||
fi | |||
|
|||
if [[ -n "${BITBUCKET_BRANCH+set}" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be elif
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, what is the +set
for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@westonruter The fi
closes out the if statement on 78. This is essentially the fall back (when the environment variables TRAVIS
and BITBUCKET_BRANCH
are not set) which was on the original repo at 72-75.
As for +set
, I was under the impression that this was needed to check if a variable was set in bash (I'm no bash master), instead of checking against a value. If it's incorrect I will gladly update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the structure should be:
if [ "$TRAVIS" == true ]; then
# ...
elif [[ -n "${BITBUCKET_BRANCH+set}" ]]; then
# ...
else
# ...
fi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The +set
doesn't mean anything as far as I know. I think what you mean is:
[[ -n "${BITBUCKET_BRANCH}" ]]
Or equivalently:
[[ ! -z "${BITBUCKET_BRANCH}" ]]
As currently the -n "${BITBUCKET_BRANCH+set}"
test will never be true, because "${BITBUCKET_BRANCH+set}"
is interpolated as "+set"
when the variable is empty, at least on Bash 3.2 on OSX.
Please make these changes and update the submodule in your PR to confirm they have the desired effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@westonruter Thanks for that explanation. That makes sense. The syntax above makes complete sense as well, I should have gone that route to begin with.
The statement [[ -n "${BITBUCKET_BRANCH+set}" ]]
seemed to be working, but it could be on a newer version of Bash or could have adverse effects on Travis builds.
elif [[ ! -z "${BITBUCKET_BRANCH}" ]]; then
works just as well in my testing.
Things are updated and tested and they look to be working well on my end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The [[ -n "${BITBUCKET_BRANCH+set}" ]]
statement will seem to work because it will always work. By that I mean, it will always be true, since "+set"
is not null.
@@ -69,10 +69,14 @@ function set_environment_variables { | |||
DIFF_BASE=${DIFF_BASE:-$TRAVIS_COMMIT^} | |||
fi | |||
DIFF_HEAD=${DIFF_HEAD:-$TRAVIS_COMMIT} | |||
elif [[ ! -z "${BITBUCKET_BRANCH}" ]]; then | |||
DIFF_BASE=${DIFF_BASE:-$BITBUCKET_COMMIT^} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@glewiswp note that this will only compare with the previous commit which will lead to misleading results. See #197 and #229 for the same problem and a a proposed workaround for the same issue in Travis branch builds. What is needed here is the branch that the user intends to merge the changes into in order to be able to get the proper $DIFF_BASE
. On Travis CI pull requests, this is available as $TRAVIS_BRANCH
where it is defined as being the destination branch for where the changes will be merged. Nevertheless, for BitBucket it seems that the $BITBUCKET_BRANCH
variable is not the pull request destination branch but rather the current branch, and thus it will have the exact same problem as Travis CI branch builds.
This PR introduces Bitbucket Pipelines support as discussed in #232.
Fixes #232.