-
Notifications
You must be signed in to change notification settings - Fork 235
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
Add autosync before local change for new and edit commands #307
base: main
Are you sure you want to change the base?
Add autosync before local change for new and edit commands #307
Conversation
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.
LGTM! Simple and well-written, thanks Patrik!
Could you also look into adding tests for this if it's not too much of a hassle? The codebase is still lacking testing wise, so I try to add tests every time I touch a feature.
I don't think we have tests for sync in general yet, so it'll be a nice challenge to see how we'll mock the clients to make them testable locally! Good practice in refactoring code to make it testable.
Yep I can do that |
I'm new to open source so I'm happy to practice a bit on a smaller project |
Let's say I want to test |
Yes exactly. You need to parametrize these dependencies, then you'll be able to replace them with your mocks. By making Client an argument, you can then bring in our own TestClient and pass that in instead and be able to test the logic without calling a real client! I would've linked a nice article on dependency inversion and injection, but I couldn't find one that wasn't worded in a complicated way, and I think you've got the idea honestly. |
Issue #, if available:
#57
Description of changes:
Currently, snippets are lost when autosync is enabled and snippets are added from different machines.
With this change, auto sync is called both before and after a new snippet is added to the local file. This means remote changes will not be overwritten.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.