Skip to content
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 release version in footer #1183

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Fix release version in footer #1183

merged 1 commit into from
Sep 26, 2023

Conversation

deborahchua
Copy link
Contributor

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.

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.

Before:
Screenshot 2023-09-25 at 14 37 33

After (tested in Integration):
Screenshot 2023-09-25 at 14 38 09

Trello card: https://trello.com/c/uoKDUcbV/3182-fix-broken-version-footer-in-publishing-apps-2

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

@deborahchua deborahchua marked this pull request as ready for review September 25, 2023 14:31
Copy link
Contributor

@AgaDufrat AgaDufrat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and nice idea of using the SENTRY_RELEASE!
What do you think of only showing a short SHA (first 10 characters) like it was before?

@deborahchua
Copy link
Contributor Author

Looks good and nice idea of using the SENTRY_RELEASE!

I cannot take the credit - this was @robinjam 's suggestion! 🥇

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 deborahchua merged commit 553fff4 into main Sep 26, 2023
8 checks passed
@deborahchua deborahchua deleted the fix-footer-version branch September 26, 2023 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants