-
Notifications
You must be signed in to change notification settings - Fork 319
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
Skip “Percy screenshots” when secrets are unavailable on forks (fix) #3075
Conversation
767e5cd
to
93eca04
Compare
a92fae6
to
a6a3cd4
Compare
339def3
to
2f1c6f8
Compare
2f1c6f8
to
8986fb2
Compare
@@ -18,7 +18,14 @@ jobs: | |||
PERCY_TOKEN: ${{ secrets.PERCY_TOKEN }} | |||
PORT: 8888 | |||
|
|||
# Skip when secrets are unavailable on forks | |||
if: ${{ !github.event.pull_request.head.repo.fork }} |
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.
For our typical "PRs from forks" scenario this line skips the Percy screenshots
steps: | ||
- name: Check secrets | ||
if: ${{ !env.PERCY_TOKEN }} | ||
run: echo "::warning title=GitHub Actions secrets::Workflow requires 'PERCY_TOKEN' secret" |
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.
Ready for review again @domoscargin |
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.
Nice, I think this looks good now!
Looks like our "local branches only" check for Percy screenshots didn't work
I'd forgotten that workflows for fork PRs identify our repo making this check always pass: 🙈
❌ Use the
github.repository_owner
context✅ Use the
github.event
context to identify forksFor example, this PR (fork) has passed the incorrect check and tried to run Percy
But others (my fork) use the new check and Percy has been skipped:
main
) to confirm Percy is skipped #3129Unlike the old fix, Percy is not skipped when run directly from the fork:
This allows contributors to add their own
${{ secrets.PERCY_TOKEN }}
but with a warning if not provided: