Anton Gilgur
Anton Gilgur
Ohhh, good catch! I totally missed that in the `root.go`, especially since it says `ARGO_BASE_HREF` on the very next line. Sorry I misread your first comment! Thanks for clarifying and...
> https://github.com/argoproj/argo-workflows/blob/main/cmd/argo/commands/root.go#L87C102-L87C111 Ah I totally missed this column number in the link -- I was on mobile where it's far off screen to the right so I didn't see the...
> https://github.com/argoproj/argo-workflows/blob/main/cmd/argo/commands/root.go#L87C102-L87C111 > > Yea let me modify that too. I could've sworn I searched for all references to `BASE_HREF` so let me double check that too if I missed...
Yes, all downstream consumers that use this, like the Helm Charts, will need to be updated. That is the nature of breaking changes.
Merged `main` for new PR checks per #13027
Going to merge this as Tianchu approved and Bala LGTM'd and there have been no reviews from others in the past ~4 months
> @agilgur5, seems this change will break the helm chart, it still refers to BASE_HREF. See: https://github.com/argoproj/argo-helm/blob/2f82fb5992fe1e390d1ebdbc4be6d5d6c6549a37/charts/argo-workflows/templates/server/server-deployment.yaml#L98 Since `ARGO_BASE_HREF` already exists, I addressed this in the chart for forward compat:...
> Since `ARGO_BASE_HREF` already exists, I addressed this in the chart for forward compat: [argoproj/argo-helm#2756](https://github.com/argoproj/argo-helm/pull/2756) Annnd I didn't re-read my own release notes or this PR (it's 4 months old,...
> Why not combining with #12775? Well, to begin with, that one was made a few days beforehand (both made 2+ weeks ago), and has now been merged earlier too....
Merged `main` for new PR checks per #13027