Skip to content

Switch to extension.open for opening the marketplace#15321

Merged
jakebailey merged 3 commits into
microsoft:mainfrom
jakebailey:replace-extension-open
Feb 8, 2021
Merged

Switch to extension.open for opening the marketplace#15321
jakebailey merged 3 commits into
microsoft:mainfrom
jakebailey:replace-extension-open

Conversation

@jakebailey
Copy link
Copy Markdown
Member

For #14264

Thanks to microsoft/vscode#115344 (comment), we don't need to use open with the schema hack. This command does the same thing.

@jakebailey jakebailey changed the title Switch to open.extension for opening the marketplace Switch to extension.open for opening the marketplace Feb 5, 2021
@jakebailey jakebailey requested a review from rchiodo February 5, 2021 00:07
Copy link
Copy Markdown

@karrtikr karrtikr left a comment

Choose a reason for hiding this comment

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

LGTM

@jakebailey
Copy link
Copy Markdown
Member Author

jakebailey commented Feb 5, 2021

This will fail due to a circular dependency:

Activating extension 'ms-python.python' failed:
Circular dependency found:
Symbol(IInterpreterSecurityService)
--> Symbol(IInterpreterSecurityStorage)
--> Symbol(ICommandManager)
--> Symbol(IJupyterExtensionDependencyManager)
--> Symbol(ICommandManager).

I tested with the Pylance ones, then saw the Jupyter ones after and fixed them, but didn't test locally.

@jakebailey
Copy link
Copy Markdown
Member Author

Solved it my making it a parameter, the cycle was that CommandManager and JupyterExtensionDependencyManager depended directly on each other. Arguably, a better fix is to move the install logic to the command manager, as it is the only user, but that's a more significant change.

@jakebailey jakebailey merged commit 5264b20 into microsoft:main Feb 8, 2021
@jakebailey jakebailey deleted the replace-extension-open branch February 8, 2021 19:00
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.

4 participants