deis / workflow-cli

The CLI for Deis Workflow
http://deis.com
MIT License
31 stars 43 forks source link

fix(Jenkinsfile): set git_branch to env.BRANCH_NAME #277

Closed vdice closed 7 years ago

vdice commented 7 years ago

Our logic for discerning the git branch was operating on the assumption that it would either be remotes/origin/master or a PR branch. In fact, if the HEAD commit on the master branch is tagged, it will resolve to tags/<tag>.

As the logic above is used in many spots, this changes over to relying on env.BRANCH_NAME, which will be master even if tagged.

Tested by porting this change manually to the master job to success: https://ci.deis.io/job/Deis/job/workflow-cli/job/master/173/

(With previous code, CI was executing logic on the assumption that it was a PR - kicking off pr test job, not building certain binaries, etc. - now we see it following the intended master path)

Fixes https://github.com/deis/workflow-cli/issues/276

deis-bot commented 7 years ago

@Joshua-Anderson, @helgi and @mboersma are potential reviewers of this pull request based on my analysis of git blame information. Thanks @vdice!

codecov-io commented 7 years ago

Current coverage is 72.60% (diff: 100%)

No coverage report found for master at 9a26cb3.

Powered by Codecov. Last update 9a26cb3...351d15e

vdice commented 7 years ago

Actually, realizing this isn't particular to codecov. Anywhere we check the git branch in the binary fashion of either 'remotes/origin/master' or <PR> needs to be updated... (and same in controller-sdk-go)

bacongobbler commented 7 years ago

So with this change, what is the expected workflow when we make a PR to workflow-cli? Since master is hardcoded, will the PR job always fail until someone tells CI to build the PR as the expected branch?

If it's a matter of us uploading our reports to codecov as remotes/origin/master, would it be easier to strip remotes/origin/ before uploading?

vdice commented 7 years ago

@bacongobbler PRs will still work as they have been (you can see the job triggered by this PR here).

This just changes to using the Jenkins Pipeline-provided env.BRANCH_NAME for resolving the branch name (it will be master for master and PR-<number> for a PR) instead of using the git cli to do so.