Skip to content

[DO NOT MERGE][ci] Run FPGA manufacturing tests on all PRs - #14

Merged
pamaury merged 1 commit into
lowRISC:mainfrom
pamaury:pr_ot_ci
Sep 8, 2026
Merged

[DO NOT MERGE][ci] Run FPGA manufacturing tests on all PRs#14
pamaury merged 1 commit into
lowRISC:mainfrom
pamaury:pr_ot_ci

Conversation

@pamaury

@pamaury pamaury commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

The mechanism used is the cross-repo ci infrastructure defined by lowRISC which enables triggering the Opentitan CI from other repositories.

Important note: in order to get a token with sufficient permissions, the FPGA CI workflow is a pull_request_target, meaning that on a PR, it runs with the content of the workflow on the main branch (not with the content in the PR!). For this reason, I put the FPGA workflow test in a different file.

Requires lowRISC/opentitan#30802

DO NOT MERGE yet: the current branch to trigger is current a testing branch on the opentitan repo, the final version should use earlgrey_1.0.0 when lowRISC/opentitan#30802 is merged

@pamaury
pamaury force-pushed the pr_ot_ci branch 5 times, most recently from d559f86 to fdb8251 Compare August 17, 2026 19:43
@pamaury pamaury changed the title [ci] Run FPGA manufacturing tests on all PRs [DO NOT MERGE][ci] Run FPGA manufacturing tests on all PRs Aug 24, 2026

@AlexJones0 AlexJones0 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good - I'd appreciate if you can address/answer my one comment before merging (and this should wait for #16 to merge first anyway and be rebased on top of that).

Comment thread .github/workflows/fpga-ci.yml Outdated
with:
script: |
const payload = {
sha: context.eventName === 'pull_request' ? context.payload.pull_request.head.sha : context.sha,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does this not need to be:

Suggested change
sha: context.eventName === 'pull_request' ? context.payload.pull_request.head.sha : context.sha,
sha: context.eventName === 'pull_request_target' ? context.payload.pull_request.head.sha : context.sha,

(if not, why? Repeated twice below if so.)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes good catch, I changed from pull_request to pull_request_target to get a more privileged token and forgot to update this

Comment thread .github/workflows/fpga-ci.yml Outdated
event_type: 'cross-repo-ci',
client_payload: {
...payload,
target: `${context.repo.owner}/opentitan/ot-sku-ci/ot-sku-ci.yml`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems good, just don't forget to change this to earlgrey_1.0.0 before merging :)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done :)

The mechanism used is the cross-repo ci infrastructure defined by
lowRISC which enables triggering the Opentitan CI from other
repositories.

Signed-off-by: Amaury Pouly <amaury.pouly@opentitan.org>

@AlexJones0 AlexJones0 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM if you're happy that all requisite changes have been merged.

@pamaury
pamaury merged commit d297f99 into lowRISC:main Sep 8, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants