-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[CI] Make premerge upload/write comments #166609
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
base: main
Are you sure you want to change the base?
[CI] Make premerge upload/write comments #166609
Conversation
Created using spr 1.3.7 [skip ci]
Created using spr 1.3.7
|
@llvm/pr-subscribers-github-workflow Author: Aiden Grossman (boomanaiden154) ChangesThis only does this for Linux currently as the issue-write workflow Full diff: https://github.com/llvm/llvm-project/pull/166609.diff 2 Files Affected:
diff --git a/.github/workflows/issue-write.yml b/.github/workflows/issue-write.yml
index 26cd60c070251..8966a1b372dfc 100644
--- a/.github/workflows/issue-write.yml
+++ b/.github/workflows/issue-write.yml
@@ -7,6 +7,7 @@ on:
- "Check for private emails used in PRs"
- "PR Request Release Note"
- "Code lint"
+ - "CI Checks"
types:
- completed
diff --git a/.github/workflows/premerge.yaml b/.github/workflows/premerge.yaml
index 973d3abf358ce..2b88ebd3e6106 100644
--- a/.github/workflows/premerge.yaml
+++ b/.github/workflows/premerge.yaml
@@ -116,6 +116,13 @@ jobs:
path: artifacts/
retention-days: 5
include-hidden-files: 'true'
+ - name: Upload Comment
+ if: always()
+ continue-on-error: true
+ with:
+ name: workflow-args
+ path: |
+ comments
premerge-checks-windows:
name: Build and Test Windows
|
|
We can't just upload a single comment from one job, because the Linux/Windows/Linux AArch64 jobs will race against each other and we have no ordering guarantees. |
This only does this for Linux currently as the issue-write workflow currently does not support writing out multiple comments. This gets the ball rolling as the failures that most people see are common to both platforms. Ensuring we have coverage on Windows for comments will be done in a future patch. Pull Request: llvm#166609
Created using spr 1.3.7 [skip ci]
This only does this for Linux currently as the issue-write workflow currently does not support writing out multiple comments. This gets the ball rolling as the failures that most people see are common to both platforms. Ensuring we have coverage on Windows for comments will be done in a future patch. Pull Request: llvm#166609
This only does this for Linux currently as the issue-write workflow currently does not support writing out multiple comments. This gets the ball rolling as the failures that most people see are common to both platforms. Ensuring we have coverage on Windows for comments will be done in a future patch. Pull Request: llvm#166609
Created using spr 1.3.7 [skip ci]
This only does this for Linux currently as the issue-write workflow currently does not support writing out multiple comments. This gets the ball rolling as the failures that most people see are common to both platforms. Ensuring we have coverage on Windows for comments will be done in a future patch. Pull Request: llvm#166609
Created using spr 1.3.7 [skip ci]
| with: | ||
| name: workflow-args | ||
| path: | | ||
| comments |
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.
Where is this file being created and is it only created on x86?
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.
It's created in https://github.com/llvm/llvm-project/blob/main/.ci/premerge_advisor_explain.py which gets called by the monolithic-* scripts. But yes, only on x86 currently. Update to only try the upload on x86 so that we do not start failing the AArch64 job.
tstellar
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.
This LGTM.
Does the comment write job get triggered once per job or once per workflow? If it's once per workflow, you could add a final job at the end of the workflow that collects comments from the 3 CI jobs and then uploads them together as a single comment.
Ack. I was thinking of doing it by making |
|
I don't really have strong opinions about how to do it. That was just one idea I remember trying in the past. |
We want the premerge advisor to write out comments, and we need the issue-write workflow to trigger on it in order for this to work. Landing this before the rest of #166609 to enable testing that given this needs to be in repo due to permissions issues.
This only does this for Linux currently as the issue-write workflow
currently does not support writing out multiple comments. This gets the
ball rolling as the failures that most people see are common to both
platforms. Ensuring we have coverage on Windows for comments will be
done in a future patch.