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

Use "positron" as the publisher for our extensions #5597

Merged
merged 6 commits into from
Dec 4, 2024

Conversation

juliasilge
Copy link
Contributor

@juliasilge juliasilge commented Dec 4, 2024

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 be positron.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:

Screenshot 2024-12-03 at 5 27 04 PM

@juliasilge juliasilge marked this pull request as ready for review December 4, 2024 00:53
@@ -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') {
Copy link
Contributor Author

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.

Copy link
Collaborator

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.

@juliasilge juliasilge requested a review from jmcphers December 4, 2024 00:56
jmcphers
jmcphers previously approved these changes Dec 4, 2024
Copy link
Collaborator

@jmcphers jmcphers left a 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') {
Copy link
Collaborator

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.

Co-authored-by: Jonathan <[email protected]>
Signed-off-by: Julia Silge <[email protected]>
@juliasilge juliasilge merged commit 9605ef0 into main Dec 4, 2024
25 checks passed
@juliasilge juliasilge deleted the positron-publisher-field-for-extensions branch December 4, 2024 23:32
@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2024
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