-
Notifications
You must be signed in to change notification settings - Fork 204
Ensure Piped uses QUICK_SYNC for deployment triggered on OUT_OF_SYNC #2795
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
Conversation
|
Code coverage for golang is
|
|
web-test seems to be failed 👁️ |
|
Code coverage for golang is
|
|
Thanks. I have just fixed the test. |
|
Nice |
| // Avoid triggering multiple deployments for the same application in the same iteration. | ||
| if _, ok := triggered[app.Id]; ok { | ||
| continue | ||
| } |
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.
Looks like the trigger which comes first has a higher priority than the one of the same application which comes after. I'm okay with that but since those candidates come from different kinds (onCommand, onCommit, onOutOfSync, etc), maybe we need a better way to choose which one should have a higher priority based on the c.Kind or something.
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.
Yes. I understand your feeling. 👍
Tbh, I was thinking about that. One way I was planned to use is sorting the candidate list in this function by kind, but after the second thought, I decided to do nothing because the passing list could be controlled by the caller. The caller can sort in the order it wants.
Maybe in the future, when we have more kinds we can add some kind of priority field to the candidate struct to decide.
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.
Okay, let's keep it as is and make changes to use ordering later.
|
Nice work 👍 |
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: