-
Notifications
You must be signed in to change notification settings - Fork 92
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
Use "positron" as the publisher for our extensions #5597
Conversation
@@ -114,7 +114,7 @@ suite('Python Runtime Session', () => { | |||
} as Partial<JupyterAdapterApi>) as JupyterAdapterApi; | |||
|
|||
sinon.stub(vscode.extensions, 'getExtension').callsFake((extensionId) => { | |||
if (extensionId === 'vscode.kallichore-adapter' || extensionId === 'vscode.jupyter-adapter') { |
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.
@jmcphers was this maybe an oversight that this still said vscode.kallichore-adapter
? I want to highlight that I have changed it here to positron.positron-adapter
.
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.
It's an oversight, but should be positron.positron-supervisor
.
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.
LGTM, modulo positron.positron-supervisor
change. Thanks for taking care of this!
@@ -114,7 +114,7 @@ suite('Python Runtime Session', () => { | |||
} as Partial<JupyterAdapterApi>) as JupyterAdapterApi; | |||
|
|||
sinon.stub(vscode.extensions, 'getExtension').callsFake((extensionId) => { | |||
if (extensionId === 'vscode.kallichore-adapter' || extensionId === 'vscode.jupyter-adapter') { |
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.
It's an oversight, but should be positron.positron-supervisor
.
extensions/positron-python/src/test/positron/session.unit.test.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Jonathan <[email protected]> Signed-off-by: Julia Silge <[email protected]>
Addresses #5268
After experimenting with this, I believe we should only update the publisher on extensions that we ourselves truly did make, not ones that we forked and made changes to. As an example, the Black formatter (which we bundle) depends on the
ms-python.python
extension ID; if we change our Python extension to bepositron.python
then we can no longer bundle the Black formatter.QA Notes
Most importantly, there should be no errors as the app comes up about extensions being activated, and both R and Python runtimes should start.
There are more implications for changing this publisher in the long term as outlined in #5268, but to confirm we have successfully updated it, you can look for some of the builtin extensions in the Extensions tab and see that they say they have the "positron" publisher: