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
Added configuration module #39
Conversation
Codecov upload limit reached
|
c41d008
to
63f9012
Compare
@daniellemaxwell please do a |
{ | ||
"Porthole": { | ||
"ensignUrlConfig": { | ||
"tenantBaseUrl": "process.env.REACT_APP_TENANT_BASE_URL", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could be misunderstanding, but doesn't this set the tenantBaseUrl
configuration to the string "process.env.REACT_APP_TENANT_BASE_URL"
? I thought the point of the config
module was that reasonable defaults could be set and then the configuration could be loaded from the environment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's correct. I should've checked about this and the quarterdeckBaseUrl
as I wasn't sure if we wanted to set them to the URLs listed in SC-9979 or list a custom environment as I did here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@daniellemaxwell ok, well since the config module isn't being used yet - should we go ahead and merge this and take next steps later?
@bbengfort That's fine. We can also talk about which environment variables may need to be added to the configuration docs. |
Scope of changes
Added configuration module using node-config per the instructions on this page: https://github.com/node-config/node-config/wiki/Environment-Variables#custom-environment-variables
Fixes SC-9979
Type of change
Acceptance criteria
Describe how reviewers can test this change to be sure that it works correctly. Add a checklist if possible.
Author checklist
Reviewer(s) checklist