Skip to content
This repository was archived by the owner on Jul 15, 2023. It is now read-only.

Gate the check for gopls updates using a setting #2773

Merged
merged 4 commits into from
Oct 17, 2019

Conversation

ramya-rao-a
Copy link
Contributor

@ramya-rao-a ramya-rao-a commented Sep 21, 2019

As seen in #2768 and #2767, the implicit calls to https://proxy.golang.org/golang.org/x/tools/gopls/@v/list can fail in multiple scenarios.

In VS Code, we also want to follow the guideline of not making any outgoing calls over the network unless it is done as part of a feature resulting from an explicit user action which the user is aware of.

Exceptions to this rule should be documented and a setting should be provided for the user to opt in/out.

Keeping this in mind, the recent feature of making calls to https://proxy.golang.org/golang.org is being updated as follows in this PR

  • Provide a setting to opt in to the automatic update check. This is similar to the extensions.autoCheckUpdates setting in VS Code
  • If the above is not enabled, then hard-code the latest known latest version of gopls in the extension

@ramya-rao-a
Copy link
Contributor Author

cc @stamblerre

@@ -79,7 +80,7 @@ export async function registerLanguageFeatures(ctx: vscode.ExtensionContext) {

// The user may not have the most up-to-date version of the language server.
const tool = getTool(toolName);
const update = await shouldUpdateLanguageServer(tool, path);
const update = await shouldUpdateLanguageServer(tool, path, config.checkForUpdates);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just:

if config.checkForUpdates {
    const update = await shouldUpdateLanguageServer(tool, path);
    if (update) {
        promptForUpdatingTool(toolName); 
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd still want to check against the version that is hard-coded in the extension if the setting is disabled.
How about renaming the setting to go.checkGoProxyForToolUpdates?
For now, we can use it only for gopls.
In the future the same approach can be taken for other tools.

@ramya-rao-a ramya-rao-a merged commit a1856f6 into microsoft:master Oct 17, 2019
@ramya-rao-a ramya-rao-a deleted the hard-code-latest-gopls branch October 17, 2019 02:53
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants