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

Never show again storage keys are registered too late for syncing #93817

Closed
sandy081 opened this issue Mar 31, 2020 · 2 comments
Closed

Never show again storage keys are registered too late for syncing #93817

sandy081 opened this issue Mar 31, 2020 · 2 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug settings-sync verified Verification succeeded
Milestone

Comments

@sandy081
Copy link
Member

sandy081 commented Mar 31, 2020

Never show again storage keys are registered for syncing after the notification is shown. This is too late because the corresponding state is only synced (pulled) after the keys are registered and because of this these notifications are shown on other machine.

For example try out

  • On first machine, Install a language pack on one machine
  • When the prompt comes, choose not to show it again
  • On a second machine, enable sync

🐛 Prompt is shown

@bpasero
Probably this a new kind of key for which state has to be synced always. I would probably introduce a new kind of keys called non-versioned which gets synced always. It means you do not need to provide version while registering.

Edit: I do not think above solution works as it only fixes for the dialog which will be shown after. Existing dialogs which are asked not to be shown will not be synced because their keys will never get registered. All do not ask keys shall be registered outside notification service by respective owners always.

@sandy081 sandy081 added bug Issue identified by VS Code Team member as probable bug settings-sync labels Mar 31, 2020
@sandy081 sandy081 added this to the March 2020 milestone Mar 31, 2020
@bpasero
Copy link
Member

bpasero commented Mar 31, 2020

Yeah good catch, instead of registering from within the notification service, we need to move this out. That is unfortunate, because we will constantly have to make sure that these keys are synced once new ones are being added...

@sandy081
Copy link
Member Author

Yeah that's necessary for syncing. For a moment I was tempted to have ignored list (other way around) like in settings, but I am worried that what if someone introduce a state storing paths or some sensitive info and forget to register it as ignored state.

@sandy081 sandy081 removed their assignment Apr 2, 2020
@sandy081 sandy081 added the verified Verification succeeded label Apr 2, 2020
@github-actions github-actions bot locked and limited conversation to collaborators May 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug settings-sync verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

2 participants