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

Use synchronize option to sync settings #211

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

WilsonZiweiWang
Copy link
Contributor

@WilsonZiweiWang WilsonZiweiWang commented May 1, 2024

The synchronize option is marked as deprecated but what we did to sync the settings was not a good practice either. The recommended pull-model for configurations is demonstrated here. However, this model doesn't really fit our case as the logging level needs an immediate update but others can be fetched when needed.

One of the advantages of using the pull-model is it can specify the scope (user/workspace/workspaceFolder) of the configurations before fetching.

@WilsonZiweiWang WilsonZiweiWang self-assigned this May 1, 2024
@WilsonZiweiWang WilsonZiweiWang marked this pull request as draft May 1, 2024 18:19
@WilsonZiweiWang WilsonZiweiWang changed the title Use synchronize to sync settings Use synchronize option to sync settings May 2, 2024
@WilsonZiweiWang WilsonZiweiWang removed their assignment May 4, 2024
@WilsonZiweiWang WilsonZiweiWang force-pushed the Sync-settings branch 2 times, most recently from cfe3e1a to 3f0c5fc Compare June 12, 2024 14:28
Though this option is marked as deprecated, we can't fully embrach the recommended pull-model for syncing configuration
as the logger's logging level can't be synced in the same way like others. It needs to be updated immediately
instead of being fetched from the client when needed.
More info: https://code.visualstudio.com/api/language-extensions/language-server-extension-guide#explaining-the-language-server
@WilsonZiweiWang WilsonZiweiWang marked this pull request as ready for review June 20, 2024 19:36
Copy link
Member

@deribaucourt deribaucourt left a comment

Choose a reason for hiding this comment

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

I tested updating the path from an erroneous one to a proper one, server features seemed fine like embedded documents linting which seems to go through this.

@deribaucourt deribaucourt merged commit 904fe79 into yoctoproject:staging Jul 17, 2024
5 checks passed
@deribaucourt deribaucourt deleted the Sync-settings branch July 17, 2024 11:50
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.

2 participants