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

.github/workflows: verify that each commit builds for test suite changes #16556

Merged
merged 1 commit into from Jun 21, 2021
Merged

.github/workflows: verify that each commit builds for test suite changes #16556

merged 1 commit into from Jun 21, 2021

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Jun 16, 2021 •
edited

Make sure the individual commits of a PR build without errors for test
suite changes as well.

Noticed during review of #16470, see #16470 (comment)

@tklauser
.github/workflows: verify that each commit builds for test suite changes
ce18f20 
Make sure the individual commits of a PR build without errors for test
suite changes as well.

Noticed during review of #16470, see
#16470 (comment)

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser added the area/CI-improvement Topic or proposal to improve the Continuous Integration workflow label Jun 16, 2021
@tklauser tklauser requested review from a team as code owners June 16, 2021 12:12
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 16, 2021
@tklauser tklauser requested a review from nbusseneau June 16, 2021 12:13
@tklauser tklauser added the release-note/ci This PR makes changes to the CI. label Jun 16, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 16, 2021
@tklauser tklauser mentioned this pull request Jun 16, 2021
Merged
nbusseneau
nbusseneau approved these changes Jun 18, 2021
View reviewed changes
Copy link
Member

@nbusseneau nbusseneau left a comment

Choose a reason for hiding this comment

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

Code changes LGTM. Any idea of the time it takes to build for, let's say, 10 commits? /cc @pchaigno also who has context on this workflow.

pchaigno
pchaigno approved these changes Jun 18, 2021
View reviewed changes
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Thanks! One less TODO on my side 🎉

To answer Nicolas's question, building the test/ code is pretty fast (significantly faster than make build or make -C build_all). So with the filter in place, I don't expect a big impact on our queue.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 18, 2021
@jibi jibi merged commit e260ba9 into master Jun 21, 2021
@jibi jibi deleted the pr/tklauser/gh-action-build-commits-test-suite branch June 21, 2021 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pchaigno pchaigno pchaigno approved these changes

@christarazi christarazi christarazi approved these changes

@nbusseneau nbusseneau nbusseneau approved these changes

Assignees
No one assigned
Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tklauser @pchaigno @christarazi @nbusseneau @jibi