-
Notifications
You must be signed in to change notification settings - Fork 28
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
fix: use master/production on merge #329
Conversation
7fee035
to
21be168
Compare
Codecov Report
@@ Coverage Diff @@
## master #329 +/- ##
=======================================
Coverage 94.50% 94.50%
=======================================
Files 27 27
Lines 3000 3000
Branches 161 161
=======================================
Hits 2835 2835
Misses 147 147
Partials 18 18
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
.github/workflows/ci.yml
Outdated
export PUBLISH_TAGS=${{ github.head_ref }} | ||
export GIT_ENV='development' | ||
else | ||
export PUBLISH_TAGS=${GITHUB_REF:11} |
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.
Why can't we use github.ref
directly here?
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.
Same.
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.
oh, it looks like we can use $GITHUB_REF directly as well. https://docs.github.com/en/actions/learn-github-actions/environment-variables
Removing the above declaration.
One nit to be answered mentioned by Azan. Otherwise, it's good. |
21be168
to
8a5675e
Compare
.github/workflows/ci.yml
Outdated
export PUBLISH_TAGS=${{ github.head_ref }} | ||
if [ $IS_MERGED == false ] | ||
then | ||
export PUBLISH_TAGS=${{ github.head_ref }} |
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 believe we also have GITHUB_HEAD_REF
, can we use it for sake of consistency?
8a5675e
to
cd7fceb
Compare
PROD-2552
The previous job merge job failed due to
github.head_ref
not givingmaster
on merge.Using the same method as we did in VEM using github.ref on merge action gives you the branch name.