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.

Sign up for GitHub

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

Jump to bottom

ci: refactor workflows #275

Merged
merged 6 commits into from
Jul 24, 2023
Merged

ci: refactor workflows #275

merged 6 commits into from
Jul 24, 2023

Conversation

katexochen
Copy link
Contributor

Some refactoring of workflows:

  • Remove invalid filter on create trigger. This trigger doesn't allow filters.
  • Only use push trigger on main (wasn't used before anyway, as there was an if statement on most jobs).
  • Adding path filters to image-rs and ocicrypt-rs workflows.
  • Removing if statements on jobs. The workflow triggers control when a job run, no need to filter with an if statement on a single job.
  • Trying to unify naming a bit, fixing white space issues.
  • Adding workflow_dispatch trigger to every test workflow, so they can be triggered manually where needed (like on forks).

@katexochen katexochen requested a review from sameo as a code owner July 11, 2023 10:56
portersrc
portersrc reviewed Jul 11, 2023
View reviewed changes
.github/workflows/image_rs_build.yml Outdated
- 'main'
paths:
- 'image-rs/**'
- '.github/image_rs_build.yml'
Copy link
Member

@portersrc portersrc Jul 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Hey @katexochen , I'm not strong with github workflows, but should this be .github/workflows/image_rs_build.yml? Similar thing for .github/workflows/ocicrypt_rs_build.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thanks for catching this. :)

Xynnn007
Xynnn007 approved these changes Jul 13, 2023
View reviewed changes
Copy link
Member

@Xynnn007 Xynnn007 left a comment

Choose a reason for hiding this comment

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

Much thanks for this @katexochen ! The CI now looks much better. Only some personal questions

.github/workflows/aa_cc_kbc.yml Outdated
@@ -1,6 +1,8 @@
name: CC kbc build CI
on:
push:
branches:
- "main"
Copy link
Member

Choose a reason for hiding this comment

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

I found others are 'main'. Does this matter? s.t. ' and "

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it makes a difference, but I updated it anyway for consistency.

.gitignore
@@ -4,3 +4,4 @@ Cargo.lock
.DS_Store

image-rs/scripts/attestation-agent
shell.nix
Copy link
Member

Choose a reason for hiding this comment

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

Is this something left when test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is part of my local development setup, so I added to .gitignore.

@arronwy arronwy mentioned this pull request Jul 13, 2023
Merged
arronwy
arronwy approved these changes Jul 24, 2023
View reviewed changes
Copy link
Member

@arronwy arronwy left a comment

Choose a reason for hiding this comment

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

Thanks @katexochen LGTM!

@Xynnn007
Copy link
Member

Can you rebase this PR? @katexochen

@katexochen
ci: remove invalid filters on create trigger
e3708e0 
Signed-off-by: Paul Meyer <49727155+katexochen@users.noreply.github.com>
@katexochen
ci: only trigger push on main
b9e33f2 
Signed-off-by: Paul Meyer <49727155+katexochen@users.noreply.github.com>
@katexochen
ci: update image-rs and ocicrypt-rs triggers
d088c4d 
Signed-off-by: Paul Meyer <49727155+katexochen@users.noreply.github.com>
@katexochen
ci: remove ifs on jobs
48632c8 
Signed-off-by: Paul Meyer <49727155+katexochen@users.noreply.github.com>
@katexochen
ci: update workflow names and path filters
9afb35a 
Signed-off-by: Paul Meyer <49727155+katexochen@users.noreply.github.com>
@katexochen
ci: add workflow_dispatch trigger to tests
5b5d68c 
Signed-off-by: Paul Meyer <49727155+katexochen@users.noreply.github.com>
@arronwy arronwy merged commit 814d34e into confidential-containers:main Jul 24, 2023
21 checks passed
@katexochen katexochen deleted the fix/ci-trigger branch July 24, 2023 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@portersrc portersrc portersrc left review comments

@arronwy arronwy arronwy approved these changes

@Xynnn007 Xynnn007 Xynnn007 approved these changes

@sameo sameo Awaiting requested review from sameo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@katexochen @Xynnn007 @portersrc @arronwy