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

github/ci: New CI framework #30921

Merged
merged 7 commits into from
Dec 9, 2023
Merged

github/ci: New CI framework #30921

merged 7 commits into from
Dec 9, 2023

Conversation

phlax
Copy link
Member

@phlax phlax commented Nov 16, 2023

README: https://github.com/envoyproxy/envoy/blob/48afe3a868c3a175c393d5feca285cf1345a6c7a/.github/workflows/README.md

config: https://github.com/envoyproxy/envoy/blob/48afe3a868c3a175c393d5feca285cf1345a6c7a/.github/config.yml

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@phlax phlax changed the title github/ci: New CI framework [WIP] github/ci: New CI framework Nov 16, 2023
@phlax phlax marked this pull request as draft November 16, 2023 13:11
ci/do_ci.sh Outdated Show resolved Hide resolved
@phlax phlax force-pushed the gh-new-ci branch 3 times, most recently from 9a15cd9 to d79e5e8 Compare December 6, 2023 16:06
@phlax phlax changed the title [WIP] github/ci: New CI framework github/ci: New CI framework Dec 6, 2023
@phlax phlax marked this pull request as ready for review December 6, 2023 16:40
wbpcode
wbpcode previously approved these changes Dec 7, 2023
Copy link
Member

@wbpcode wbpcode left a comment

Choose a reason for hiding this comment

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

Blinded LGTM. Although I had a talk with @phlax offline, it's still almost impossible for me to give a reasonable review to this super large PR.

The annoying point it that we can only verify the CI after we merge it. Anyway, let's do it.

@phlax
Copy link
Member Author

phlax commented Dec 7, 2023

The annoying point it that we can only verify the CI after we merge it

that will always be the case using non pull_request events (i covered this a bit in the README) - im happy to demo how anything works if that is helpful in the staging repo

REMOVE.txt Outdated Show resolved Hide resolved
@phlax
Copy link
Member Author

phlax commented Dec 7, 2023

@wbpcode ive had a go at simplifying the filters in _finish.yml and the upstream ci/request action - hopefully they are a bit cleaner/clearer

@abeyad
Copy link
Contributor

abeyad commented Dec 8, 2023

@phlax I scanned the mobile bits of the PR and it looks reasonable to me, but agree with @wbpcode that it's hard to review it thoroughly and understand how it works compared to previous CI. Pretty great if RBE works with this!

@phlax
Copy link
Member Author

phlax commented Dec 8, 2023

it's hard to review it thoroughly and understand how it works compared to previous CI

i added a README to try and cover the headline bits - but we can defo document this better as we go

abeyad
abeyad previously approved these changes Dec 8, 2023
phlax added 5 commits December 9, 2023 09:29
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
@phlax phlax merged commit 4b8eeb7 into envoyproxy:main Dec 9, 2023
30 of 40 checks passed
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.

4 participants