refactor(stores): implement Store → Settings bidirectional sync #216
Conversation
|
/gemini review |
Summary of ChangesHello @KubrickCode, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where changes made within the application's store were not being automatically saved to VS Code settings, leading to data loss. It refactors the store synchronization logic to establish a robust bidirectional sync, ensuring that any modifications to buttons, button sets, or the active set are consistently persisted to the configured VS Code settings or local storage. The changes include a new queuing mechanism for saves, improved error reporting, and support for different configuration scopes. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a bidirectional synchronization between the application's store and VS Code settings, which is a crucial feature to prevent data loss. The implementation of a queueSave pattern to manage consecutive updates and prevent race conditions is well-executed, and the accompanying tests are thorough, covering various scenarios including error handling and circular dependency prevention. My review includes a few suggestions to enhance the implementation, primarily focusing on resolving a potential race condition in the save logic, improving adherence to the coding style guide, and reducing minor code duplication. Overall, this is a well-structured and important update.
KubrickCode
force-pushed
the
develop/shlee/194-4
branch
from
5b7b44e to
071609c
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements bidirectional synchronization between the application's store and VS Code settings, which is a great improvement to prevent data loss. The changes include adding a save queue to handle consecutive updates gracefully, showing error messages on failures, and handling different configuration scopes.
My review focuses on improving adherence to the coding style guide and ensuring robust error handling. I've pointed out a few places where magic strings and numbers can be replaced with constants, and a scenario where a silent failure could occur.
Overall, the implementation is solid and the new tests provide good coverage for the new functionality.
Settings were lost because Store changes were not auto-saved to VS Code Settings - Add queueSave pattern to prevent race conditions on consecutive updates - Show vscode.window.showErrorMessage on save failures - Throw explicit error when configWriter is missing to prevent silent failures - Branch save logic by LOCAL/WORKSPACE/GLOBAL scope fix #194
KubrickCode
force-pushed
the
develop/shlee/194-4
branch
from
071609c to
0c62f4e
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces bidirectional synchronization between the application store and VS Code settings, addressing the issue where store changes were not automatically saved. The changes include a queueSave pattern to handle consecutive updates, explicit error handling for missing configWriter, and branching save logic based on the configuration scope (LOCAL/WORKSPACE/GLOBAL). The new tests comprehensively cover the bidirectional sync for buttons, button sets, and active set, as well as circular update prevention and error handling. The dispose method now correctly unsubscribes from store listeners, which is a good improvement for resource management. The implementation generally adheres to the repository's coding conventions, particularly regarding error handling and modularization.
|
🎉 This PR is included in version 0.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Settings were lost because Store changes were not auto-saved to VS Code Settings
fix #194