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.

Sign up for GitHub

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

Jump to bottom

[chart]: remove VERSION envvar from default and declare explicitly #5175

Merged
merged 4 commits into from
Aug 12, 2021
Merged

[chart]: remove VERSION envvar from default and declare explicitly #5175

merged 4 commits into from
Aug 12, 2021

Conversation

mrsimonemms
Copy link
Contributor

Fixes #5023

Found four references to this in application, all in the JS app (looked in Golang but found nothing)

server
ws-manager-bridge
payment-endpoint
db-sync

Also, have removed the noVersion flag from the proxy deployment

@mrsimonemms mrsimonemms marked this pull request as ready for review August 12, 2021 14:59
@csweichel
[ws-manager-bridge] Remove VERSION
4e744cd 
logging is not a good enough reason to restart bridge so often
@csweichel
[db-sync] Remove VERSION env var
1414201 
logging is not a good enough reason to restart this service
@csweichel
[payment-endpoint] Remove VERSION env var
ccaa78f 
Logging is not a good enough reason to restart this service
@csweichel
Copy link
Contributor

/lgtm

@roboquat roboquat added the lgtm label Aug 12, 2021
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: b539e6029ab31beaf3b281c8618ec23266d62030

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csweichel

Associated issue: #5023

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit df20a0a into main Aug 12, 2021
@roboquat roboquat deleted the sje/remove-version-envvar branch August 12, 2021 16:00
@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented Aug 13, 2021 •
edited
Loading

It's not just process.env to look for - sometimes its getEnvVar. See #5187 and #5189

@ArthurSens ArthurSens mentioned this pull request Dec 1, 2021
Merged
ArthurSens added a commit that referenced this pull request Dec 1, 2021
@ArthurSens
Revert #5175
7ef17db 
Signed-off-by: ArthurSens <arthursens2005@gmail.com>
ArthurSens added a commit that referenced this pull request Dec 2, 2021
@ArthurSens
Revert #5175
e4148c0 
Signed-off-by: ArthurSens <arthursens2005@gmail.com>
ArthurSens added a commit that referenced this pull request Dec 2, 2021
@ArthurSens
Revert #5175
a637391 
Signed-off-by: ArthurSens <arthursens2005@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akosyakov akosyakov Awaiting requested review from akosyakov

@csweichel csweichel Awaiting requested review from csweichel

@meysholdt meysholdt Awaiting requested review from meysholdt

Assignees

@csweichel csweichel

Labels
approved size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Don't pass VERSION env var into every container
3 participants
@mrsimonemms @csweichel @roboquat