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
fix: parsed url is not exposed (#11816) #11916
Conversation
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
Codecov ReportBase: 47.29% // Head: 47.29% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #11916 +/- ##
=======================================
Coverage 47.29% 47.29%
=======================================
Files 245 245
Lines 41665 41665
=======================================
Hits 19706 19706
Misses 19974 19974
Partials 1985 1985 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
I suggest a more subtle fix like this, as it won't hide possible future unrelated errors on other method calls: let parsed;
try {
parsed = GitUrlParse(url);
} catch {
// URL Parsing failed
return null;
} |
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
@alexef could I trouble you for some unit tests on this function? :-) |
Also, am I correct to only cherry-pick this to 2.6? |
@crenshaw-dev yes, this is 2.6 only. I'll add the unit tests |
@crenshaw-dev nevermind, the tests already exists: https://github.com/argoproj/argo-cd/blob/master/ui/src/app/shared/components/urls.test.ts#L7 now the question is why they didn't fail when the previous change was introduced that changed the scope of |
@alexef good question... I'll leave that as an exercise for later, since this fixes an important issue. :-) |
Cherry-picked onto release-2.6 for 2.6.0-rc3. |
@alexef May I ask why you set the |
@jannfis the UI was completely broken (unable to view application details, presented stacktrace, no workaround) in master and in the 2.6 branch until rc3. If this doesn't grant a |
@alexef Ah. Just as a heads-up for the future, the |
* fix: parsed url is not exposed (argoproj#11816) Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * subtler fix, thanks @woehrl01 Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
* fix: parsed url is not exposed (argoproj#11816) Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * subtler fix, thanks @woehrl01 Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> Signed-off-by: schakrad <chakradari.sindhu@gmail.com>
follow up on: #11819 according to @RomanBats
fixes: #11816
fixes: #11922
Now the code ofnot anymore after subtler fixrevisionURL
is consistent withrepoURL
above it.Signed-off-by: Alex Eftimie alex.eftimie@getyourguide.com
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist: