-
Notifications
You must be signed in to change notification settings - Fork 10
github actions: Allow fork checkout for commit validation #743
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
Conversation
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.
Pull request overview
This PR modifies the validate-kernel-commits.yml workflow to support checking out pull requests from forked repositories, enabling validation of external contributors' code. The changes add explicit repository and ref parameters to the checkout action that reference the PR's head repository.
Key changes:
- Added
repositoryparameter to checkout action to specify the fork repository - Changed
refparameter to use PR event context for consistency with fork checkouts
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c653f1d to
692dd27
Compare
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
692dd27 to
13e92c5
Compare
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bmastbergen
left a comment
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.
LGTM 🥌
13e92c5 to
802053c
Compare
802053c to
afa671a
Compare
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Branch names and repository names can contain special characters. Quote all such expressions used in shell scripts.
a6c5837 to
b8f67ee
Compare
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b8f67ee to
c592d3c
Compare
c592d3c to
28d00af
Compare
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
28d00af to
06a1d92
Compare
|
Its finally working |
|
JIRA check confirmed to work on local branch: https://github.com/ctrliq/kernel-src-tree/actions/runs/19975988193/job/57292178451?pr=749 I think this PR is ready to |
kerneltoast
left a comment
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.
bmastbergen
left a comment
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.
🥌
There are many workflows that are useful without the JIRA checks from Forks. We're still evaluating how to best deal with this without a lot of engineer overhead to check external contributors.
7dc7867
06a1d92 to
7dc7867
Compare
|
Cleaned up copilot feedback that came on my other PR even though copilot didn't mention it in this PR: #750 (comment) |
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bmastbergen
left a comment
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 approve Sultans commits but I cannot approve the pr |
kerneltoast
left a comment
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.
Approval for Maple's ![]()

Since we have external contributors we need process each of the commits for faster reviews, validation that header information is correct and to check the state of our tickets so that is actually tracked correctly internally, on behalf of some external contributors.
This is a failure PR to checkout PR: https://github.com/ctrliq/kernel-src-tree/actions/runs/19444908687/job/56799122710?pr=704
This is it checking out the code but failing due no secrets, to be handled separately:
https://github.com/ctrliq/kernel-src-tree/actions/runs/19943135283/job/57185843398?pr=742
Many Updates Later
Thanks to copilot known more about GHA's than I do there has been a bit of a refactor.
PRIOR, we were using the GH
actions/checkout@v4to check out the PR first, then fetching the base_ref so we have the names to the sha's since finding the parent branch is a little annoying in git.This worked fine for local branches, but when we start enabling FORK branch we checking out that users version of the kernel src tree. So a bit of a reorder is needed.
First Step:
Second Step:
actions/checkout@v4to assure ourselves that the base repo is CIQ's repo.This in turn causes a set of potential failure conditions doing it as if we were working in a blank local repo ... see copiolots review: github actions: Allow fork checkout for commit validation #743 (comment)