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: Add pipeline change detection #24221

Merged
merged 2 commits into from
Nov 29, 2022
Merged

Conversation

phlax
Copy link
Member

@phlax phlax commented Nov 28, 2022

Signed-off-by: Ryan Northey ryan@synca.io

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:]

part of #23758

@phlax phlax force-pushed the ci-change-detection branch 5 times, most recently from aa62874 to 289f752 Compare November 28, 2022 11:08
@phlax phlax changed the title ci: Add pipeline change detection [WIP] ci: Add pipeline change detection Nov 28, 2022
@phlax phlax marked this pull request as draft November 28, 2022 12:45
.azure-pipelines/pipelines.yml Outdated Show resolved Hide resolved
.azure-pipelines/pipelines.yml Outdated Show resolved Hide resolved
@alyssawilk alyssawilk self-assigned this Nov 28, 2022
@alyssawilk
Copy link
Contributor

hah I was about to ask if it was easier for you to pick this up than suggest all the fixes. Thanks so much for tackling it! LMK when this is ready as I'd love to get something in this week

@phlax phlax force-pushed the ci-change-detection branch 4 times, most recently from 99bdff5 to 5ce1ea9 Compare November 29, 2022 10:47
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Member Author

phlax commented Nov 29, 2022

i have not added any actual exclusions to this PR - just the framework for detecting changes - as when we add any exclusions we need to think carefully about how it will run on main or other release branches and on tagged commits/releases

im inclined to not exclude anything when a PR lands - this does risk PRs passing and then failing when they land - but i would rather that than commits passing and then releases not passing and i think we need to ensure every change is tested somewhere

@phlax
Copy link
Member Author

phlax commented Nov 29, 2022

testing PR is #24204

@phlax phlax changed the title [WIP] ci: Add pipeline change detection ci: Add pipeline change detection Nov 29, 2022
@phlax phlax marked this pull request as ready for review November 29, 2022 11:50
@phlax
Copy link
Member Author

phlax commented Nov 29, 2022

this is probably gtg altho we may need to iterate - testing is hard because the PR introduces change itself

@alyssawilk
Copy link
Contributor

I'd be inclined to land with at least one CI we think is affected (i.e. https://github.com/envoyproxy/envoy/pull/24204/files) as I'm not sure landing this as-is is going to tell us anything. WDYT?

@phlax
Copy link
Member Author

phlax commented Nov 29, 2022

WDYT?

adding a change here is probably not going to help as the PR contains change itself

i think land this and then run the test PR a couple of times, and then clarify what should/not be excluded

@phlax
Copy link
Member Author

phlax commented Nov 29, 2022

adding a change here is probably not going to help as the PR contains change itself

hmm, actually thinking about it - yep i see - we need at least one thing to be excluded to test - ill add ...

@alyssawilk
Copy link
Contributor

yeah I'll also say now we have this (or almost) I am so excited to say not run all the Envoy tests for docs only changes. This is going to be pretty awesome :-D

Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Member Author

phlax commented Nov 29, 2022

k - ive added a condition which hopefully will only run coverage all checks iff either:

  • commit is postsubmit (on any branch)
  • the PR is neither docs-only nor mobile-only

should work

Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

Let's see how it goes :-)

@phlax phlax enabled auto-merge (squash) November 29, 2022 14:11
@phlax phlax merged commit b3aa08d into envoyproxy:main Nov 29, 2022
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.

None yet

2 participants