-
Notifications
You must be signed in to change notification settings - Fork 137
Introduce envconfig package
#1795
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
Conversation
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.
Mostly questions. Not sure I have enough context to approve
| ConfigDataSource, | ||
| } from './types'; | ||
|
|
||
| export { fromTomlConfig, fromTomlProfile, toTomlConfig, toTomlProfile, loadConfigData } from './utils'; |
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.
Do we really want to expose all of those functions publicly? If so, we'll also need to expose the Toml* types.
But I'd preach to avoid conflating the public API surface at this point.
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. We have followed a pattern across SDKs to allow users to convert to/from TOML representations on their own.
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.
In that case, we'll need to also export the Toml* interfaces and remove the @internal annotation on those.
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.
A few more details; see comments. But ok to merge once you've handled those.
What was changed
Created
envconfigpackage, functionality that allows users to configureConnectionorNativeConnectionbacked clients (or just the connection itself for that matter).Why?
Allows users to configure connections/clients from config files instead of source code.
Closes [Feature Request] Environment Configuration #1727
How was this tested:
Integration test suite at
test-envconfig.tsAny docs updates needed?
Yes