-
Notifications
You must be signed in to change notification settings - Fork 2
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
Load .env files when launching from CLI #133
Conversation
Tested this a bit more thoroughly, it feels stable to me! I pulled any recommendation for using |
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.
This looks great! Thanks for the PR. Only had one question about loadConfig()
as defined in .docs/generate.mjs
. Looks like maybe that's a relic?
Also, should we explicitly mark credentials.js
as deprecated (the class and constructor itself)?
Great addition to the library, and will offer a lot more flexibility.
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.
Great! This looks good to merge to me. @claytercek Do you think we're at a good point to publish to NPM now? Also wondering if this actually constitutes to a minor change for all packages (minus scaffold) instead of just a patch at this point.
Deprecated the Credentials class and all its methods. Unfortunately, there's no way to mark the |
Oh yeah, this probably makes more sense as a minor change. I'll update that and merge. I think once this is merged it would be in a good spot to publish! |
resolves #53
I refactored the
ConfigManager
to just be a couple of exported functions fromconfig.js
, as now that it's not a singleton, basically all of the methods were just static anyways.