Conversation
| PR_NUMBER_AND_COMMIT_SHA: ${{ github.event.number }}-${{ github.event.pull_request.head.sha }} | ||
| # Since this only runs on a merge into main, we can't use github.event.number | ||
| # so we instead use the word "beta" and the PR merge commit SHA | ||
| PR_NUMBER_AND_COMMIT_SHA: beta-${{ github.sha }} |
There was a problem hiding this comment.
I don't love using "beta" where you would expect a PR_NUMBER but this was the least invasive change I could think of. I didn't want to have to add another environment variable and make this more complicated than it needs to be.
There was a problem hiding this comment.
But then again, we could always change this to NPM_VERSION but I don't know if that will make it more complicated. Open to ideas. Just wanted to open a fix quickly.
There was a problem hiding this comment.
My bad, I reviewed too quickly and missed this comment I think 😛
In case this is still relevant, I think it is a valid concern and NPM_VERSION would describe what this variable is for perfectly.
Codecov Report
@@ Coverage Diff @@
## main #4781 +/- ##
=======================================
Coverage 69.18% 69.18%
=======================================
Files 29 29
Lines 1652 1652
Branches 363 363
=======================================
Hits 1143 1143
Misses 432 432
Partials 77 77 Continue to review full report at Codecov.
|
|
✨ Coder.com for PR #4781 deployed! It will be updated on every commit.
|
|
@im-coder-lg no, different fix. That should also be fixed though! |
3 participants
I didn't think it through in my previous PR but since this workflow only runs on a merge into
main, there is no PR number. So instead, we use the word "beta" and then we also have to grab a different SHA since there is no PR SHA.Prevents this from happening again: https://www.npmjs.com/package/code-server/v/4.0.1--
https://github.com/coder/code-server/runs/4903294989?check_suite_focus=true#step:3:16
Fixes N/A