build(gha): Fix js lint job for forked repos #21893
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joshuarli
reviewed
Nov 10, 2020
| @@ -19,6 +19,8 @@ jobs: | |||
| # because we want to lint + fix + commit, we need to checkout the HEAD sha (otherwise | |||
| # it checks out the merge commit and we will not be able to commit to it) | |||
| ref: ${{ github.event.pull_request.head.ref || 'master' }} | |||
| # We need the repo here so that this works for forks | |||
| repo: ${{ github.event.pull_request.head.repo.full_name }} | |||
There was a problem hiding this comment.
Wait, don't we need pull_request_target à la #21600 as well?
There was a problem hiding this comment.
We don't because there are no secrets (this doesn't include github_token since it will use the token from the forked repo) that are used, and no write access to getsentry/sentry repo is needed.
There was a problem hiding this comment.
You can see it working here #21906
There was a problem hiding this comment.
@joshuarli Updated to have a specific task for forks as the action doesn't have access to write to the fork :/
billyvg
commented
Nov 11, 2020
| @@ -85,8 +92,7 @@ jobs: | |||
| git push origin | |||
|
|
|||
| - name: tsc | |||
| if: steps.changes.outputs.frontend == 'true' | |||
| continue-on-error: true | |||
There was a problem hiding this comment.
This is not what we want (it means if tsc fails, it moves on to next step without failing job).
This fixes the js lint job for forked repos, otherwise it would attempt to check out the forked branch from getsentry repo.
billyvg
force-pushed
the
build/gha/fix-js-lint-for-forks
branch
from
eddbd01 to
53158f3
Compare
joshuarli
approved these changes
Nov 11, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes the js lint job for forked repos, otherwise it would attempt to check out the forked branch from getsentry repo. We can't run the eslint + fix on forks, so create a new step for forks.