Skip to content

Remove unnecessary activationEvents from extension's package.json#2815

Merged
norascheuch merged 2 commits intomainfrom
nora/remove-unused-activation-events
Oct 12, 2023
Merged

Remove unnecessary activationEvents from extension's package.json#2815
norascheuch merged 2 commits intomainfrom
nora/remove-unused-activation-events

Conversation

@norascheuch
Copy link
Copy Markdown
Contributor

From VS Code version 1.74.0 onwards, we no longer need some activations events.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@norascheuch norascheuch requested a review from a team as a code owner September 14, 2023 11:45
Copy link
Copy Markdown
Member

@koesie10 koesie10 left a comment

Choose a reason for hiding this comment

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

To do this, we first need to ensure we only support VS Code version 1.74.0 and up. I've placed a link in the internal issue to the process we need to follow to do that.

@norascheuch norascheuch force-pushed the nora/remove-unused-activation-events branch from 076d15e to f53ecde Compare October 12, 2023 10:00
@norascheuch norascheuch requested a review from koesie10 October 12, 2023 10:23
Comment thread extensions/ql-vscode/package.json Outdated
},
"activationEvents": [
"onLanguage:ql",
"onLanguage:ql-summary",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this also be removed? It seems like this is also included in our contributes.languages.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed! In https://code.visualstudio.com/api/references/activation-events#onLanguage it's stated at some point. Thank you!
It's 'interesting' that we activate the extension when the evaluator-log.summary is opened (why?..) by defining a 'language' that is connected to this file.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems like we do have some language support for summary files, so this is probably the easiest way to actually make VS Code use that language.

Copy link
Copy Markdown
Member

@koesie10 koesie10 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@norascheuch norascheuch enabled auto-merge October 12, 2023 11:51
@norascheuch norascheuch merged commit f4f7998 into main Oct 12, 2023
@norascheuch norascheuch deleted the nora/remove-unused-activation-events branch October 12, 2023 12:03
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