-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Remove jupyter functionality in favor of Jupyter extension #302
Conversation
Archive of 0.7.0
* 'master' of https://github.com/Microsoft/vscode-python: Fixes #56 list all environments (#219) Fixes #57 Disable activation on debugging (#220) Fixes #26 Do not run linters when linters are disabled (#222)
* upstream/master: Fix typo in README.md (#252) Disable linter without workspaces (#241)
* upstream/master: Fix feedback service (#246) Fix django context initializer (#248) disable generation of tags file upon extension load (#264)
* upstream/master: Resolve pythonPath before comparing it to shebang (#273)
* upstream/master:
Fixes #22 to Detect anaconda from known locations (#221)
Use workspaceFolder token instead of workspaceRoot (#267)
Fix registry lookup response (#224)
Fix issues when running without debugging and debugged code terminates (#249)
}; | ||
|
||
export interface IFeatureDeprecationManager { | ||
notifyDeprecationOfJupyter(): void; |
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.
So does this mean we will have to come up with a new method name for every feature we want to deprecate? Could we configure a dictionary or array to contain the info about deprecations and then have a method that we call which goes through the data structure as necessary at startup?
Or could we make this more specific by checking if the Jupyter extension is already installed to justify the separate method? I guess my key question is whether we want to blindly go through the deprecations at startup and have the logic of whether to do something here or to keep the details at the call site.
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.
So does this mean we will have to come up with a new method name for every feature
Absolutely not.
Could we configure a dictionary or array to contain the info ..as necessary at startup?
Yes, thats the plan. That's what I've done with formatOnSave and will do for lintOnTextChange, etc..
Jupyter is different. Here we need to first check if the other Jupyter
extension is installed, and if not, then bind to those jupyter commands (add command handlers).
I didn't want to push all of that information into the IFeatureDeprecationManager, felt it shouldn't have the logic to check if jupyter
extension is installed or not.
More than happy to change, cuz that way all code related to deprecation is in one place. But the conditional code to check if jupyter is installed will be copied to two places and the idea of binding to the commands feel icky...
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.
Lol, writing all of that makes me want to go back and move everything into the same place.
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 make the changes, hang in there 😃
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.
Happy to play the role of rubber ducky for you. 😄
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.
Fixed
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 introduce changes to check for deprecated settings in separate PR.
@brettcannon 🙏 , want to avoid having to copy and paste code for other PRs 😃 |
Fixes #223
Display message informing user of the Jupyter extension when making use of deprecated jupyter functionality.
Common code to launch browser window.
Manager class for notifications of deprecated features.
Remove jupyter code and releated unit tests, except for one class that provides information for the separate Jupyter extension.