-
Notifications
You must be signed in to change notification settings - Fork 552
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
Settings v2 #5815
Settings v2 #5815
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 am really excited to see this new settings system come to life!
Given our recent conversation I felt compelled to comment early, despite the draft status and hope you don't mind.
Thank you, this was really helpful. I have now implemented your feedback |
All these cool kids using Arc 😆 |
Applying Byron's suggestion to make the ownership clearer
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 went 'all in' on the review and hope I left it more maintainable than before. With that said, please feel free to dial it back here and there. Particularly the names might be overly verbose and could just be simpler.
Please do all the application settings real-world testing once again, I only launched the app once and it seemed to work, but I didn't do any app-setting specific testing.
- add docs according to what I think it does - make settings access blocking to avoid spurious but avoidable failures. - rename `SettingsHandle` to `AppSettingsWithDiskSync`, and additional renames to make clear what's happening. - make interacting with the app-settings more pleasant. - more robust watcher event loop
The broader goal here is to create a good setup for application settings that is ergonomic for users, easy to work with and not error prone. In general, what VSCode does with settings is great and probably a good target - a user can use the UI or edit the JSON, the settings are portable.
Requirements:
Outstanding questions:
~/.config/gitbutler/settings.json
,~/.gitbutler.json
? Somewhere else? How about Windows?TODOs:
Out of scope here:
LegacySettings
and the tauri store plugin