-
Notifications
You must be signed in to change notification settings - Fork 734
deps(vscode): temporarily de-bump min back to 1.68.0 #5747
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
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1046,6 +1046,6 @@ | |
| }, | ||
| "engines": { | ||
| "npm": "^10.1.0", | ||
| "vscode": "^1.83.0" | ||
| "vscode": "^1.68.0" | ||
| } | ||
| } | ||
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,15 +7,48 @@ import type { ExtensionContext } from 'vscode' | |
| import { activate as activateCore, deactivate as deactivateCore } from 'aws-core-vscode/node' | ||
| import { awsToolkitApi } from './api' | ||
| import { Commands } from 'aws-core-vscode/shared' | ||
| import * as semver from 'semver' | ||
| import * as vscode from 'vscode' | ||
| import { telemetry } from 'aws-core-vscode/telemetry' | ||
|
|
||
| export async function activate(context: ExtensionContext) { | ||
| await activateCore(context) | ||
|
|
||
| // after toolkit is activated, ask Amazon Q to register toolkit api callbacks | ||
| await Commands.tryExecute('aws.amazonq.refreshConnectionCallback', awsToolkitApi) | ||
| void setupVscodeVersionNotification() | ||
| return awsToolkitApi | ||
| } | ||
|
|
||
| export async function deactivate() { | ||
| await deactivateCore() | ||
| } | ||
|
|
||
| // TODO: remove once version bump to 1.83.0 is complete. | ||
| export function setupVscodeVersionNotification() { | ||
|
||
| let notificationDisplayed = false | ||
| tryShowNotification() | ||
|
|
||
| function tryShowNotification() { | ||
| // Do not show the notification if the IDE version will continue to be supported. | ||
| if (!semver.gte(vscode.version, '1.83.0')) { | ||
| return | ||
| } | ||
|
|
||
| if (notificationDisplayed) { | ||
| return | ||
| } | ||
|
|
||
| notificationDisplayed = true | ||
|
|
||
| telemetry.toolkit_showNotification.emit({ | ||
| component: 'editor', | ||
| id: 'versionNotification', | ||
| reason: 'unsupportedVersion', | ||
| result: 'Succeeded', | ||
| }) | ||
| void vscode.window.showWarningMessage( | ||
| 'Update VS Code to version 1.83.0+, support for previous versions will be dropped soon. ' | ||
| ) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this ever run? Its always set to false on line 111.
Also it seems its only called once on activation anyways. This seems like more code than we need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I honestly wasn't sure, but I was using this as an example:
aws-toolkit-vscode/packages/amazonq/src/app/chat/activation.ts
Lines 77 to 107 in 7d6a975
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That example sorta makes sense because the state of
notificationDisplayedcan change after its initialized but before evaluation. But regardless it is a such a confusing and unnecessary way to evaluate the message. IMO we should refactor that.