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: minor refactor to build-images job #1611

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

willfindlay
Copy link
Contributor

We don't want to run make version in the context of a PR branch, so run it from master before checking out the PR branch.

We don't want to run `make version` in the context of a PR branch, so run it from master
before checking out the PR branch.

Signed-off-by: William Findlay <will@isovalent.com>
@willfindlay willfindlay requested a review from a team as a code owner October 18, 2023 14:32
@willfindlay willfindlay added area/ci Related to CI release-note/ci This PR makes changes to the CI. labels Oct 18, 2023
Copy link
Contributor

@ferozsalam ferozsalam left a comment

Choose a reason for hiding this comment

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

👍 this looks better to me, thanks

Copy link
Contributor

@kevsecurity kevsecurity left a comment

Choose a reason for hiding this comment

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

LGTM

@willfindlay willfindlay merged commit 577dfe6 into main Oct 18, 2023
28 of 29 checks passed
@willfindlay willfindlay deleted the pr/willfindlay/refactor-build-images-job branch October 18, 2023 15:53
@@ -59,17 +59,26 @@ jobs:
echo "tag=${{ github.sha }}" >> $GITHUB_OUTPUT
fi

- name: Checkout Source Code
- name: Checkout master branch
Copy link
Member

Choose a reason for hiding this comment

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

nit

Suggested change
- name: Checkout master branch
- name: Checkout main branch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah nice catch. PR's already merged, but let's fix this in a follow-up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci Related to CI release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants