Fix release version in footer #2375
Conversation
There was a problem hiding this comment.
Thanks so much for making this change.
If I'm following the code correctly, I think this means that there will be a slight change in behaviour in development (i.e. when SENTRY_RELEASE is not defined) - the footer will now display "Version: " instead of "Version: development". Was that an intentional change? If so, it would be good to explain that in the commit message / PR description.
An alternative implementation preserving the original behaviour would be something like:
<% content_for :footer_version, ENV.fetch("SENTRY_RELEASE", "development") %>I'm happy to leave the above to your discretion, so I'm going to mark this as "Approved". Thanks, again!
|
For reference we are tracking this in https://trello.com/c/h19UQFNE |
This has been broken since we moved to EKS and the SHA is no longer provided. We can use the [SENTRY_RELEASE](https://github.com/alphagov/govuk-helm-charts/blob/077117e7f40c096a958f9e0b853a2bd76cf43f26/charts/generic-govuk-app/templates/deployment.yaml#L92C1-L92C1) environment variable which is defined for every app and takes its value from the container image's tag. If for whatever reason the image tag is not defined it will display `null`, which should make it clearer (instead of displaying `development`) that this data is not being pulled in successfully.
deborahchua
force-pushed
the
fix-footer-version
branch
from
3de22a0
to
c1058ac
Compare
|
Thanks @floehopper ! Your suggestion is much better and clearer to the user if there is an issue obtaining the image tag - I've updated the code and commit message. |
This has been broken since we moved to EKS and the SHA is no longer provided.
We can use the SENTRY_RELEASE environment variable which is defined for every app and takes its value from the container image's tag.
Trello cards:
https://trello.com/c/uoKDUcbV/3182-fix-broken-version-footer-in-publishing-apps-2
https://trello.com/c/h19UQFNE/269-footerversion-not-set-in-govukadmintemplate