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

Auto update notification #5884

Merged
merged 8 commits into from
Jan 13, 2025
Merged

Auto update notification #5884

merged 8 commits into from
Jan 13, 2025

Conversation

timtmok
Copy link
Contributor

@timtmok timtmok commented Jan 6, 2025

Address #1837

Checks the releases JSON and determines if there is an update available. This can be manually invoked from the cog menu at the bottom of the left sidebar or Positron will do so once per hour.

The Output view shows debug messages in the Main output. It will show the update URL (points to the platform's release JSON) at startup. Update checks are logged here as well. If the update URL is unavailable, a toast notification displays the error with more details in the Output view.

There didn't seem to be a calendar versioning package that supported build numbers and zero-padding so that's why there's a Positron version utility. It seems important to know that 2024.12.0-1 is older than 2024.12.0-2 if we want to be able to test updating private releases.

There are two environment variables for internal use:

  • POSITRON_UPDATE_CHANNEL - The default is prereleases but can be set to dailies or staging for releases that haven't been published to Positron's releases page on GitHub. dailies isn't auto-publishing at the moment and staging has a 1-day expiry.
  • POSITRON_AUTO_UPDATE - Setting to 1 will automatically download the update and notify the user when it is ready to restart to complete the install. This is for testing and needs a certified build (at least on Mac).

This will check is functional for all platforms with the following caveats:

  • Windows needs a bit of cleanup with the install type. It's going to use the system release info. I think differentiating the system and user installers via a key/value in product.json would work.
  • Linux checks the deb release info. It could use the same solution as Windows to differentiate the install type. Despite no auto-update support, it might be useful to know.
  • All platforms link to Positron's releases page on GitHub.

image
Manual check when there are no updates available

image
Update is available as indicated by the bottom of the left sidebar

The cog menu will change the check update action to download the update. This download action opens Positron's releases page in the user's browser.

QA Notes

Positron Builds can run the action to push a staging release if we want to run an internal test. This is experimental as the generated release info may be stale (rerunning doesn't automatically update the hosted file) or expired (file is auto-deleted after one day).

Copy link

github-actions bot commented Jan 6, 2025

E2E Tests 🚀  ?
This PR will run tests tagged with: @critical

@timtmok timtmok requested a review from jmcphers January 6, 2025 19:04
@timtmok timtmok marked this pull request as ready for review January 6, 2025 19:04
@@ -9,6 +9,7 @@ import { ConfigurationScope, Extensions as ConfigurationExtensions, IConfigurati
import { Registry } from '../../registry/common/platform.js';

const configurationRegistry = Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Configuration);
// --- START POSITRON ---
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit, here and elsewhere: should use the standard casing for comment fences (Start Positron)

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 keep forgetting this!

@@ -57,8 +78,11 @@ export abstract class AbstractUpdateService implements IUpdateService {
@IEnvironmentMainService private readonly environmentMainService: IEnvironmentMainService,
@IRequestService protected requestService: IRequestService,
@ILogService protected logService: ILogService,
@IProductService protected readonly productService: IProductService
// --- START POSITRON ---
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you split this into several more granular code fences? When merging, it's important that the code fences wind up in the conflicted text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense!

src/vs/platform/update/electron-main/positronVersion.ts Outdated Show resolved Hide resolved
}

protected doCheckForUpdates(context: any): void {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like this whole function from upstream got refactored out. Could you leave it in place so that it doesn't generate merge conflicts when it changes upstream? It's cool to mark it with @ts-ignore if it becomes unused.

@timtmok timtmok requested a review from jmcphers January 9, 2025 16:15
@timtmok timtmok merged commit 7f92f2a into main Jan 13, 2025
6 checks passed
@timtmok timtmok deleted the feature/auto-update-notification branch January 13, 2025 14:13
@github-actions github-actions bot locked and limited conversation to collaborators Jan 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants