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

add sha and version to ‘/‘ endpoint #574

Merged
merged 8 commits into from
Oct 15, 2024
Merged

add sha and version to ‘/‘ endpoint #574

merged 8 commits into from
Oct 15, 2024

Conversation

elrayle
Copy link
Collaborator

@elrayle elrayle commented May 1, 2024

Description

Add configurations for BUILD_SHA and APP_VERSION to use for debugging.

Related Work

Unresolved

The deploy isn't happening through the reusable workflows that would set these configs. Additionally, it is not clear how deploys are happening in the current Azure DevOps deploy. Some investigation is needed before merging.

Comment on lines +13 to +19
let version
let sha
function setup(buildsha, appVersion) {
version = appVersion
sha = buildsha
return router
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This works but I'm not sure how well I like keeping the values as global state in this file. Couldn't we just access the config directly here instead of passing as variables?
Additionally, I'm wondering if it'd be worth to still escape these values, even though they should usually be under our control.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The style matches that used in other endpoints. I'm open to other options.

@@ -23,7 +23,7 @@ jobs:
build-and-deploy:
name: Build and Deploy
needs: upload-package-lock-json
uses: clearlydefined/operations/.github/workflows/app-build-and-deploy.yml@v2.0.0
uses: clearlydefined/operations/.github/workflows/app-build-and-deploy.yml@0a6c2607470e2e0e49bf2d6636223bcd62c1a7f8
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to be changed once operations PR merged

@ljones140 ljones140 marked this pull request as ready for review October 3, 2024 17:11
APP_VERSION replaces it
@ljones140 ljones140 merged commit ef89a38 into master Oct 15, 2024
2 checks passed
@ljones140 ljones140 deleted the elr/ver-sha branch October 15, 2024 10:32
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.

4 participants