-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add cycle support #3456
Add cycle support #3456
Conversation
MDrakos
commented
Aug 20, 2024
•
edited by github-actions
bot
Loading
edited by github-actions
bot
DX-3002 Support cycles in the build plan |
Looking at the test runs it appears to be known or intermittent failures. I've tested this locally with regular checkouts and PR deploy checkouts of Go projects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should also have a test covering the Dependencies()
methods on both the Artifact and Ingredient type in the buildplan. I suspect these either need to also be cycle aware, or we'll need the test to prevent them regressing as it feels like a likely point for cycle bugs to happen.
@@ -192,3 +192,75 @@ var buildWithRuntimeDepsViaSrc = &Build{ | |||
}, | |||
}, | |||
} | |||
|
|||
var buildWithRuntimeDepsViaSrcCycle = &Build{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add a comment highlighting the cycle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Know those tests are painful to write but we're gonna be happy we have them when we have to go back to this code.