Allow users to turn off automatic checking of extension updates #55087

ramya-rao-a · 2018-07-25T19:35:10Z

Based on #54354 (comment) we abandoned the idea of a complete offline mode controlled by a single switch.

Instead, we are opting for all background features that do make a network request to be controlled via corresponding settings.

The extension update service is one such feature. This PR adds a new setting to control the extension update checks

sandy081 · 2018-07-26T14:50:34Z

src/vs/workbench/parts/extensions/electron-browser/extensions.contribution.ts

-				type: 'boolean',
-				description: localize('extensionsAutoUpdate', "Automatically update extensions"),
-				default: true,
+				type: 'string',


@ramya-rao-a This will break existing users settings file right? This will show a warning if the value is true or false.

May be we should support both types?

Supporting both types would make documenting this feature very confusing as false doesnt really mean turn off checking for updates, it only means continue to check for updates but dont install them automatically

sandy081 · 2018-07-26T14:51:31Z

src/vs/workbench/parts/extensions/electron-browser/extensionsActions.ts

+			this.enabled = configValue !== 'checkAndInstall';
+		} else {
+			this.enabled = configValue === 'checkAndInstall';
+		}


May be you should start writing the enum value instead of boolean ?

Agreed. I also removed the updateEnablement check to keep this simple. There is already a check in place to add the right action to the secondary menu in the extensions view which is good enough.

sandy081 · 2018-07-26T15:11:24Z

src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts

 	}

 	private eventuallySyncWithGallery(immediate = false): void {
+		if (this.configurationService.getValue(AutoUpdateConfigurationKey) === 'off') {


Name of this method is confusing with the new setting. This should be called checkForUpdatesAutomatically

sandy081 · 2018-07-26T15:12:00Z

src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts

@@ -414,6 +415,10 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService,
 				if (this.isAutoUpdateEnabled()) {
 					this.checkForUpdates();
 				}
+				if (!this.continuousSyncSetup && this.configurationService.getValue(AutoUpdateConfigurationKey) !== 'off') {
+					this.continuousSyncSetup = true;


Why is this variable needed ? If the value has changed you should either stop or start automatic checking?

If the config setting changed to off, then the loop for syncing is stopped. I added this to start the loop again. But on second thoughts, I can achieve the same without adding a new variable. I'll do that

sandy081 · 2018-07-26T15:15:02Z

src/vs/workbench/parts/extensions/electron-browser/extensionsViewlet.ts

@@ -452,7 +453,7 @@ export class ExtensionsViewlet extends ViewContainerViewlet implements IExtensio
 				new Separator(),
 				...(this.extensionManagementServerService.extensionManagementServers.length > 1 ? [this.groupByServerAction, new Separator()] : []),
 				this.instantiationService.createInstance(CheckForUpdatesAction, CheckForUpdatesAction.ID, CheckForUpdatesAction.LABEL),
-				...(this.configurationService.getValue(AutoUpdateConfigurationKey) ? [this.instantiationService.createInstance(DisableAutoUpdateAction, DisableAutoUpdateAction.ID, DisableAutoUpdateAction.LABEL)] : [this.instantiationService.createInstance(UpdateAllAction, UpdateAllAction.ID, UpdateAllAction.LABEL), this.instantiationService.createInstance(EnableAutoUpdateAction, EnableAutoUpdateAction.ID, EnableAutoUpdateAction.LABEL)]),
+				...((autoUpdateConfigValue === true || autoUpdateConfigValue === 'checkAndInstall') ? [this.instantiationService.createInstance(DisableAutoUpdateAction, DisableAutoUpdateAction.ID, DisableAutoUpdateAction.LABEL)] : [this.instantiationService.createInstance(UpdateAllAction, UpdateAllAction.ID, UpdateAllAction.LABEL), this.instantiationService.createInstance(EnableAutoUpdateAction, EnableAutoUpdateAction.ID, EnableAutoUpdateAction.LABEL)]),


This is the issue with changing the type and we have to carry forward this knowledge for ever. May be we can expose this value from IExtensionWorkbenchService and have only single place to know about this?

sandy081 · 2018-07-26T15:17:22Z

src/vs/workbench/parts/extensions/electron-browser/extensions.contribution.ts

+				enumDescriptions: [
+					localize('extensionsAutoUpdateCheckAndInstall', "Install extension updates automatically in the background."),
+					localize('extensionsAutoUpdateCheck', "Check for extension updates and mark extensions with available updates as outdated in the extensions view."),
+					localize('extensionsAutoUpdateOff', "No checks are made automatically for extension updates. You can still manually check for updates using the `Extensions: Check for Updates` command.")


How is "extensions.autoUpdate": "off" implies that extension update checks will not be made? How about having a new setting extension.autoCheckUpdates?

ramya-rao-a · 2018-07-26T18:23:44Z

@sandy081 I've added a new setting to simplify things.

Tyriar · 2018-07-26T21:07:28Z

How is "extensions.autoUpdate": "off" implies that extension update checks will not be made? How about having a new setting extension.autoCheckUpdates?

"extensions.autoUpdate": "off" implies that a check will not be made because it's the only value without "check" in it (and there are descriptions which explain). I'm 👎 for adding an extra setting because there are only 3 valid states, doing this doesn't make any sense and its behavior is hard to define:

"extensions.autoCheckUpdates": false
"extensions.autoUpdate": true

This discussion reminds me of the discussion around files.hotExit and we landed on this for that exact reason:

  //  - off: Disable hot exit.
  //  - onExit: Hot exit will be triggered when the last window is closed on Windows/Linux or when the `workbench.action.quit command` is triggered (command palette, keybinding, menu). All windows with backups will be restored upon next launch.
  //  - onExitAndWindowClose: Hot exit will be triggered when the last window is closed on Windows/Linux or when the `workbench.action.quit command` is triggered (command palette, keybinding, menu), and also for any window with a folder opened regardless of whether it's the last window. All windows without folders opened will be restored upon next launch. To restore folder windows as they were before shutdown set `window.restoreWindows` to `all`.
  "files.hotExit": "onExit",

ramya-rao-a · 2018-07-27T01:40:05Z

@Tyriar 2 reasons why overloading the existing setting is not a good idea from the user's prespective are

The Enable/Disable Auto Update actions convey the idea that this a feature with only 2 stages. On and off.
The most common case where users want to disable the auto-update extension is when they have installed an older version of the extension due to some issues with the latest version. When such users try to edit this setting in the settings editor, it could happen that they read the extension name "autoUpdate" and then choose "off" assuming its the auto-updates that is getting switched off. This will make them end up in a state where they get no further updates. Of course, the descriptions makes everything clear, but not everyone reads description if they feel the setting name is self explainatory.

I'm 👎 for adding an extra setting because there are only 3 valid states

That is true. When autoUpdate is true but autoCheckUpdates is false, I think its a fair compromise that the former wins.

Tyriar · 2018-07-27T02:10:00Z

it could happen that they read the extension name "autoUpdate" and then choose "off" assuming its the auto-updates that is getting switched off. This will make them end up in a state where they get no further updates.

@ramya-rao-a 👍 I'm convinced after reading this, as we want to discourage setting it to the "off" state.

ramya-rao-a · 2018-07-27T02:50:42Z

alright, then everyone here is on the same page :)

Updated PR to reflect the changes to get the new setting. Over to you @sandy081

ramya-rao-a · 2018-07-27T17:32:28Z

@sandy081 The PR now reflects your suggestion of a new setting. I'll merge it now, as we would like to have it in the Monday's Insiders. Do take a look regardless and let me know if you have any other concerns.

sandy081 · 2018-08-13T09:07:33Z

Thanks @ramya-rao-a for the changes

ramya-rao-a requested a review from sandy081 July 25, 2018 19:35

ramya-rao-a self-assigned this Jul 25, 2018

sandy081 requested changes Jul 26, 2018

View reviewed changes

ramya-rao-a force-pushed the ramyar/auto-update branch from 4ec110b to a6f5ca9 Compare July 26, 2018 18:22

ramya-rao-a changed the title ~~Overload autoupdate setting to allow complete switch off~~ Add setting to turn off automatic checking of extension updates Jul 26, 2018

ramya-rao-a force-pushed the ramyar/auto-update branch from a6f5ca9 to d8e4539 Compare July 27, 2018 01:28

ramya-rao-a changed the title ~~Add setting to turn off automatic checking of extension updates~~ Allow users to turn off automatic checking of extension updates Jul 27, 2018

ramya-rao-a force-pushed the ramyar/auto-update branch from d8e4539 to a6f5ca9 Compare July 27, 2018 02:49

ramya-rao-a added 2 commits July 26, 2018 19:51

Add setting to turn off automatic checking of extension updates

388dda0

update tag name

4520f55

ramya-rao-a force-pushed the ramyar/auto-update branch from a6f5ca9 to 4520f55 Compare July 27, 2018 02:53

Fix tests

1a41573

ramya-rao-a merged commit 5039c23 into master Jul 27, 2018

Tyriar deleted the ramyar/auto-update branch August 13, 2018 14:50

github-actions bot locked and limited conversation to collaborators Mar 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow users to turn off automatic checking of extension updates #55087

Allow users to turn off automatic checking of extension updates #55087

ramya-rao-a commented Jul 25, 2018 •

edited

Loading

sandy081 Jul 26, 2018

ramya-rao-a Jul 26, 2018

sandy081 Jul 26, 2018

ramya-rao-a Jul 27, 2018

sandy081 Jul 26, 2018

sandy081 Jul 26, 2018

ramya-rao-a Jul 26, 2018

sandy081 Jul 26, 2018

sandy081 Jul 26, 2018

ramya-rao-a commented Jul 26, 2018 •

edited

Loading

Tyriar commented Jul 26, 2018 •

edited

Loading

ramya-rao-a commented Jul 27, 2018 •

edited

Loading

Tyriar commented Jul 27, 2018

ramya-rao-a commented Jul 27, 2018 •

edited

Loading

ramya-rao-a commented Jul 27, 2018

sandy081 commented Aug 13, 2018

Allow users to turn off automatic checking of extension updates #55087

Allow users to turn off automatic checking of extension updates #55087

Conversation

ramya-rao-a commented Jul 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramya-rao-a commented Jul 26, 2018 • edited Loading

Tyriar commented Jul 26, 2018 • edited Loading

ramya-rao-a commented Jul 27, 2018 • edited Loading

Tyriar commented Jul 27, 2018

ramya-rao-a commented Jul 27, 2018 • edited Loading

ramya-rao-a commented Jul 27, 2018

sandy081 commented Aug 13, 2018

ramya-rao-a commented Jul 25, 2018 •

edited

Loading

ramya-rao-a commented Jul 26, 2018 •

edited

Loading

Tyriar commented Jul 26, 2018 •

edited

Loading

ramya-rao-a commented Jul 27, 2018 •

edited

Loading

ramya-rao-a commented Jul 27, 2018 •

edited

Loading