Skip to content

Conversation

@PlaidCat
Copy link
Collaborator

@PlaidCat PlaidCat commented Dec 2, 2025

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@v4 to 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:

  • Checkout out the CIQ Base Repo and target branch.

Second Step:

  • fetch the target branch from the CIQ repo OR the fork_repo (need to add the fork repo as a remote)
  • Then we need to checkout the PR branch since we're not longer doing that as the first step with actions/checkout@v4 to 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)

@PlaidCat PlaidCat self-assigned this Dec 2, 2025
Copilot AI review requested due to automatic review settings December 2, 2025 22:13
Copy link

Copilot AI left a 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 repository parameter to checkout action to specify the fork repository
  • Changed ref parameter to use PR event context for consistency with fork checkouts

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a 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.

Copy link

Copilot AI left a 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
bmastbergen previously approved these changes Dec 4, 2025
Copy link
Collaborator

@bmastbergen bmastbergen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🥌

Copy link

Copilot AI left a 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.
Copilot AI review requested due to automatic review settings December 5, 2025 16:22
Copy link

Copilot AI left a 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.

Copy link

Copilot AI left a 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.

@PlaidCat
Copy link
Collaborator Author

PlaidCat commented Dec 5, 2025

Its finally working
#742

@kerneltoast
Copy link
Collaborator

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 :shipit:

kerneltoast
kerneltoast previously approved these changes Dec 5, 2025
Copy link
Collaborator

@kerneltoast kerneltoast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm totally not self-reviewing anything here at all...

Image

bmastbergen
bmastbergen previously approved these changes Dec 5, 2025
Copy link
Collaborator

@bmastbergen bmastbergen left a 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.
Copilot AI review requested due to automatic review settings December 5, 2025 21:36
@kerneltoast kerneltoast dismissed stale reviews from bmastbergen and themself via 7dc7867 December 5, 2025 21:36
@kerneltoast
Copy link
Collaborator

Cleaned up copilot feedback that came on my other PR even though copilot didn't mention it in this PR: #750 (comment)

Copy link

Copilot AI left a 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.

Copy link
Collaborator

@bmastbergen bmastbergen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥌

@PlaidCat
Copy link
Collaborator Author

PlaidCat commented Dec 5, 2025

I approve Sultans commits but I cannot approve the pr :shipit:

Copy link
Collaborator

@kerneltoast kerneltoast left a 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 :shipit:

@PlaidCat PlaidCat merged commit 087b394 into main Dec 5, 2025
6 checks passed
@PlaidCat PlaidCat deleted the {jmaple}_main branch December 5, 2025 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants