Skip to content
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

Upgrade to storage.sync API, so we can synchronize Vimium settings across computers #1010

Closed
wants to merge 5 commits into from

Conversation

smblott-github
Copy link
Collaborator

Addresses #604.

@philc
Copy link
Owner

philc commented Apr 15, 2014

Nice, thank you @smblott-github for building this out and for leaving embedded refactor comments so I can follow your changes. Long time coming.

I haven't looked closely yet at understanding the full impact, but can you comment on the risk of this change? Will it break existing users' settings? Have you see any annoying behavior/conflicts when syncing across multiple computers?

@smblott-github
Copy link
Collaborator Author

Hi @philc .

This was written a long time ago. And Travis seems to be failing now. I'm sure it can be easily patched up.

"Will it break existing users' settings?":

  • It is possible. If you have two chrome/vimium instances both logged into the same account, but with different vimium settings, you end up with the settings from one or the other. Specifically, the shared settings are those from the first instance on which you click "save" on the options page. That's what triggers syncing.

"Have you see any annoying behavior/conflicts when syncing across multiple computers?"

  • No. None at all. I might have said this before, but this is in my "Could not live without" box.
  • Better still, I got a new laptop recently. Fired it up, and vimium had all my bindings and other settings configured out of the box.

If you're interested in this (and are happy with the approach), then I'd happily work on brinking it up to date.

@@ -0,0 +1,136 @@

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Black line is unnecessary

@philc
Copy link
Owner

philc commented Apr 18, 2014

Stephen, your approach looks good to me. Once the code is updated we should ship it in the next vimium release.

@smblott-github
Copy link
Collaborator Author

OK. I'll work on it.

@smblott-github
Copy link
Collaborator Author

My work on this is now over at #1029.

@philc
Copy link
Owner

philc commented Apr 30, 2014

Closing in favor of #1029

@philc philc closed this Apr 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants