-
Notifications
You must be signed in to change notification settings - Fork 59
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
Embed app config into compiled CLI binary #312
Conversation
bc9deed
to
0104d26
Compare
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.
Had a Zoom call with @kailan to review and have some things to double check before requesting a re-review.
843b9fc
to
e6c93d9
Compare
e6c93d9
to
cd8c2c9
Compare
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.
All functioning as expected on my machine! Walked through code and CI process with @Integralist and suggested some changes, which have now been implemented.
Ship it! 🚀
Problem: The logic for how to handle the local application config file, as well as how to deal with multiple blocking calls to load the config from a remote endpoint, has become complex and is prone towards hard to debug errors.
Solution: Embed the latest version of the app config into the compiled CLI binary. This means we remove two blocking remote calls, and we can failover to the embedded config whenever there is an issue trying to read the config from the user's local disk.
PLUS, we'll have tests for the config logic now! 🎉
Notes: We'll use a GitHub Action to ensure when cutting a release that the binary has the latest version (at that time) of the configuration file embedded.
Logic flow:
User.Token
/User.Email
and so that data would be lost if we switched to embedded config without asking the user first).LastChecked
andVersion
are set to the current time and current CLI version.Version
as not matching the CLI version currently running, then we'll transition them to the embedded config format (because it's possible their config might not be compatible with the updated CLI version).