-
Notifications
You must be signed in to change notification settings - Fork 107
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(ci): Use correct check for external repositories #8000
Conversation
The lint error is a known issue that shouldn't block merging this PR:
|
Failed due to #7898 |
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.
Looks good, thank you!
Not sure what happened here, the schema at that URL seems to be there.
|
It seems to be a rate-limit caused by how that action is implemented internally. |
Motivation
There's a mistake in PR #7956 where it is skipping important jobs on all PRs in CI, rather than just external PRs.
PR Author Checklist
Check before marking the PR as ready for review:
If a checkbox isn't relevant to the PR, mark it as done.
Specifications
https://docs.github.com/en/actions/learn-github-actions/contexts#github-context
You can see the full
github.event
object for an external PR here:https://github.com/ZcashFoundation/zebra/actions/runs/6997671169/job/19034900585?pr=7999#step:2:995
Solution
Use the
fork
field rather than trying to guess from the PR branch nameTesting
I manually checked this worked:
Since the logic is inverted for an external PR, that should also work.
Review
This is urgent because some PRs and Mergify are skipping some CI jobs.
Reviewer Checklist
Check before approving the PR:
PR blockers can be dealt with in new tickets or PRs.
And check the PR Author checklist is complete.