Skip to content
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

Share workflow failure alerting between ci.yml and pkgci.yml. #19444

Merged
merged 5 commits into from
Dec 11, 2024

Conversation

ScottTodd
Copy link
Member

@ScottTodd ScottTodd commented Dec 10, 2024

Progress on #9305.

Changes included:

  1. Extracts the code for parsing the results of multiple jobs and optionally posting alerts to Discord from ci.yml into a new reusable workflow in the workflow_summary.yml file.
  2. Uses the new reusable workflow in pkgci.yml.
  3. Renames to summary step to ci_summary and pkgci_summary to disambiguate "required checks". You'd think GitHub would use keys that aren't ambiguous for required checks but nooooope:
    image

@ScottTodd ScottTodd added the infrastructure Relating to build systems, CI, or testing label Dec 10, 2024
@ScottTodd ScottTodd marked this pull request as ready for review December 10, 2024 22:43
Copy link
Member

@marbre marbre left a comment

Choose a reason for hiding this comment

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

Looks pretty good but it's probably better to take a second look tomorrow 😪

.github/workflows/workflow_summary.yml Show resolved Hide resolved
.github/workflows/workflow_summary.yml Show resolved Hide resolved
Copy link
Contributor

@amd-chrissosa amd-chrissosa left a comment

Choose a reason for hiding this comment

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

Nice this looks like a great refactor!

@ScottTodd ScottTodd merged commit e6266f7 into main Dec 11, 2024
43 checks passed
@ScottTodd ScottTodd deleted the users/scotttodd/ci-alerting branch December 11, 2024 17:13
ScottTodd added a commit that referenced this pull request Dec 19, 2024
…9445)

## History

These workflow jobs were originally part of `ci.yml` but they were moved
out of part of #17957.

## Reasons to keep them separate

* Run history for each workflow is independent, e.g.
https://github.com/iree-org/iree/actions/workflows/ci_linux_x64_clang.yml
and
https://github.com/iree-org/iree/actions/workflows/ci_windows_x64_msvc.yml.
* Performance metrics for each workflow are independent:
![image](https://github.com/user-attachments/assets/5832d491-541d-46e4-9626-422f989e8879)
* Status badges are independent

## Reasons to merge them back

* Less code duplication, particularly for `setup` and `summary` steps
(see #19444)
* Less noise in the checks view in pull requests (note all the `setup`
jobs, and the PR above will add similarly dupliated `summary` jobs):
![image](https://github.com/user-attachments/assets/37628719-4d23-46f5-89ff-9da7f391695b)
* Centralized logs for checks, not split between multiple pages like
they are currently:

    Current | With this PR 
    -- | --

![image](https://github.com/user-attachments/assets/5adc117e-aa76-47a7-973f-6f1e7adcbc9c)
|
![image](https://github.com/user-attachments/assets/2cb55430-63f5-4b1f-b64c-550b919fe51f)
* This gives us a single `ci_summary` check in `ci.yml` to set as a
required check instead of multiple split across workflows
* Given enough runner capacity / budget, we could run all these jobs on
presubmit/postsubmit and not just on nightly schedules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Relating to build systems, CI, or testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants