-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add logging to App #78
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
base: main
Are you sure you want to change the base?
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
86b541a
to
5e1ba1d
Compare
5e1ba1d
to
37ee7e4
Compare
private const string ConfigSubKey = @"SOFTWARE\Coder Desktop\App"; | ||
private const string logFilename = "app.log"; | ||
#else | ||
private const string MutagenControllerConfigSection = "DebugAppMutagenController"; | ||
private const string ConfigSubKey = @"SOFTWARE\Coder Desktop\DebugApp"; | ||
private const string logFilename = "debug-app.log"; |
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.
Can these just be Coder Desktop
and Coder Desktop Debug
or something instead, rather than changing the existing key?
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.
We can do Coder Desktop Debug\App
instead of Coder Desktop\DebugApp
--- but I do want App
to be its own key to keep it distinct from the VPN service. Things like Serilog
have defined config they understand, and I think we will want to have separate control over the app and the VPN service.
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.
Yeah I didn't put two and two together about the serilog stuff, so your way is better. I don't particularly mind what key we use then, but the service keys should probably be fixed in this PR too to match the new format
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.
Any log configuration for the app should go in HKEY_CURRENT_USER
though IMO
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.
The Mutagen binary path still kinda needs to live in HKLM, since it's set by the installer. I've made it so that it starts with default config, hardcoded into the app, then checks HKLM, then HKCU.
8b98bba
to
41cf916
Compare
Adds logging to the Coder Desktop application.