search

dag-andersen / argocd-diff-preview

Tool for rendering manifest changes on pull requests.
Apache License 2.0
175 stars 10 forks source link

fix: repoURL matching is case sensitive #61

Closed SIMULATAN closed 2 weeks ago

SIMULATAN commented 2 weeks ago

this change is currently untested, I plan on conducting a full one soon

While testing this awesome project in my own repository, I found that using different casing in the application repoURL results in a confusing error message printing that the repoURL key was missing entirely. We may want to consider printing something more helpful in that case (for example, "Different 'repoURL' under spec.source in file {}")

Additionally, I fixed the debug message printing spec.sources[] as the source even when spec.source is used, which mislead me at the start. (=> rebase)

SIMULATAN commented 2 weeks ago

Ah, so that was why the CI failed. This issue reminded me once again of how complicated rust can be, foolish of me to think yolo-ing the change without a test compile would be a good idea :sweat_smile: thank you! I think I may have to rebase + autosquash as GitHub can't do it during the merge, lmk when it's time to do so.

dag-andersen commented 2 weeks ago

Ah, so that was why the CI failed. This issue reminded me once again of how complicated rust can be, foolish of me to think yolo-ing the change without a test compile would be a good idea 😅 thank you! I think I may have to rebase + autosquash as GitHub can't do it during the merge, lmk when it's time to do so.

Yes, Rust is a struggle sometimes 😄 It looks fine as it is — I’ll merge it, and it will be part of the next release ✌🏻 Thank you for the contribution! I didn’t consider that URLs could potentially include uppercase letters.

dag-andersen commented 2 weeks ago

v0.0.24 is now released 🚀 This fix is live