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

[CI-IMPROVEMENT-9] Implement check-required workflow that checks if all required checks are successful and not skipped #3369

Merged

Conversation

pavlovic-ivan
Copy link
Collaborator

@pavlovic-ivan pavlovic-ivan commented Feb 5, 2024

What type of PR is this?

This is an improvement PR

What this PR does / why we need it:

9th in a series of PRs for improvements. This implements check-required workflow that verifies if all required checks are successful, which is the case with the job in the CI workflow.

Notes for maintainers

Prerequisites:

  • Create a Github app that has permissions: "Read access to metadata" and "Read and write access to checks"
  • Create an env create-check with a protection rule for master branch with two secrets APP_ID and APP_PRIVATE_KEY and copy the apps creds created in the previous step

After merging the PR

  • update branch protection to enforce the required check to originate from the new GitHub app

Work based on these two PRs as a reference:

FIxes: #3189

Signed-off-by: Ivan Pavlovic <pavlovic.ivan.26@gmail.com>
Co-authored-by: Jonathan Giannuzzi <jgiannuzzi@users.noreply.github.com>
@pavlovic-ivan pavlovic-ivan added the PR waiting for approval PR that is finished, but needs an approval label Feb 5, 2024
@pavlovic-ivan pavlovic-ivan enabled auto-merge (squash) February 5, 2024 09:01
.addLink(check.name, check.html_url)
.addCodeBlock(JSON.stringify(check, ['status', 'conclusion', 'started_at', 'completed_at'], 2), 'json');

if (check.status === 'completed' && check.conclusion === 'success') {
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to do any handling of cases where we're not completed successfully here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This else block here is only informational. It's not necessary, but since the code is taken from FML, maybe they benefit from this info, which then makes this contribution identical to what they use. @jgiannuzzi what do you think of this? does FML benefit from it or not? should i remove it, so both are identical (making it easier to extract to separate repo later)

Copy link
Member

Choose a reason for hiding this comment

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

@dave-gantenbein any other case is handled by newCheck's default status that is set to in_progress. Basically we only create our successful check when we have found a successful check, otherwise we mark it as pending.

@pavlovic-ivan pavlovic-ivan merged commit ad0fc90 into armadaproject:master Feb 6, 2024
14 checks passed
@pavlovic-ivan pavlovic-ivan deleted the ci-improvements-part-9 branch March 11, 2024 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR waiting for approval PR that is finished, but needs an approval
Projects
None yet
5 participants