akuity / kargo

Application lifecycle orchestration
https://kargo.akuity.io/
Apache License 2.0
1.75k stars 145 forks source link

refactor: replace helm-update-image with yaml-update #2941

Closed krancour closed 5 days ago

krancour commented 6 days ago

Fixes #2939

This PR:

netlify[bot] commented 6 days ago

Deploy Preview for docs-kargo-io ready!

Name Link
Latest commit 1c0e93918854ff5d932eac3a0459223779b6c9eb
Latest deploy log https://app.netlify.com/sites/docs-kargo-io/deploys/6738ae1c05ee06000863c9ec
Deploy Preview https://deploy-preview-2941.docs.kargo.io
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov[bot] commented 6 days ago

Codecov Report

Attention: Patch coverage is 84.76190% with 16 lines in your changes missing coverage. Please review.

Project coverage is 50.92%. Comparing base (b75c455) to head (1c0e939). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/directives/yaml_updater.go 77.41% 13 Missing and 1 partial :warning:
internal/directives/helm_image_updater.go 95.34% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2941 +/- ## ========================================== + Coverage 50.86% 50.92% +0.06% ========================================== Files 278 279 +1 Lines 25113 25180 +67 ========================================== + Hits 12773 12823 +50 - Misses 11651 11666 +15 - Partials 689 691 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

krancour commented 5 days ago

Something is wrong here. The commit that undoes the helm-update-image changes is missing.

Looking into it.

krancour commented 5 days ago

Fixed now.

a20442c371ac74d652fa96ac3eebaea1e6b73be7 rolls back most recent changes to helm-update-image, but keeps non-breaking schema improvements and the additional tests that had been added.

Marvin9 commented 2 days ago

@krancour we also need to update UI as discovery mechanism of runners/registries is not automatic.

Should I replace helm-update-image with yaml-update?

krancour commented 2 days ago

@Marvin9 possible to have both for now and then remove helm-update-image in 1.2?