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

ARCH-2122 - Adding reusable workflow for validating catalog-info.yml #271

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

danielle-casella-adams
Copy link
Member

The PR creates a new reusable workflow for validating catalog-info.yml files. It has been incorporated into the workflow templates in two ways: as part of the CI build or as a standalone validation workflow.

To see it in action:

pull_request

push

The pull_request and push runs will be the same except for the step where it tries to create a PR comment. That will obviously never run if it's not a pull_request trigger.

@danielle-casella-adams danielle-casella-adams requested a review from a team as a code owner July 17, 2024 20:24
Comment on lines 41 to 50
only-validate-if-file-changed:
description: |
THIS ONLY APPLIES IF THE TRIGGER IS PULL_REQUEST OR PULL.
Flag indicating whether the validation should happen on every run or just if the catalog-info.yml file changed.
This is set to false by default, so validation will happen on every run.
If the trigger is workflow_dispatch or schedule, false is an appropriate choice because validation should always run.
For a trigger like pull_request you may want to set this to true so validation only happens if the file changed.
type: boolean
required: false
default: false
Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to get other's thoughts on this flag (especially @mike-schenk). Initially Mike indicated he wanted a smart reusable workflow that only ran if it needed to.

In this job though there are a lot of steps that have to run to determine if the validation steps in this job should run. It currently has this optional flag that makes it so the validation steps only run if the catalog-info.yml file changed, but it has to do a number of steps to examine all the applicable conditions. Since it's already doing that work, does it make sense to bail on the last two validation/reporting steps if the file didn't change? Does this flag just introduce complexity? Is it worth doing? Would turning off the failures/job summary be enough in that case?

I made a new standalone template and updated im-build-dotnet-ci with another job so they both use this reusable workflow for validating catalog-info.yml. Inside of the ci job it runs at the same time as the other jobs, so it definitely won't be the longest item. I'm not sure if the selective validation is worth it since we're doing work in the reusable job to figure out if it should run. In the standalone template, the sole point is to validate the catalog-info.yml on a scheduled basis, so it shouldn't ever skip those validation steps.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nah, I'd do the file change detection outside the reusable workflow then only invoke it conditionally.

I want to propose and promote a more efficient and comprehensive way to implement required status checks on PRs— using a singular PR-triggered workflow that just figures out which other workflows are applicable to call-out to, then fails only if any of them fail. It allows PRs that don't change anything significant to pass their required status check quickly so they can be merged without waiting for a whole battery of irrelevant tests and evaluations to run.

I've implemented something I'm pretty happy with in the funding-calculation repo, which I have just now extended to include your reusable workflow: https://github.com/im-funding/funding-calculation/pull/320

@danielle-casella-adams danielle-casella-adams merged commit 4acf5b1 into main Jul 18, 2024
@danielle-casella-adams danielle-casella-adams deleted the validate-catalog-info branch July 18, 2024 15:11
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