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

Remove jupyter functionality in favor of Jupyter extension #302

Merged
merged 19 commits into from
Nov 28, 2017

Conversation

DonJayamanne
Copy link

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.

octref and others added 16 commits November 3, 2017 13:11
* '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)
@DonJayamanne DonJayamanne added this to the December 2017 milestone Nov 28, 2017
};

export interface IFeatureDeprecationManager {
notifyDeprecationOfJupyter(): void;
Copy link
Member

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.

Copy link
Author

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...

Copy link
Author

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.

Copy link
Author

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 😃

Copy link
Member

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. 😄

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Author

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.

@DonJayamanne
Copy link
Author

@brettcannon 🙏 , want to avoid having to copy and paste code for other PRs 😃

@DonJayamanne DonJayamanne merged commit 4676284 into microsoft:master Nov 28, 2017
@DonJayamanne DonJayamanne deleted the removeJupyter branch December 12, 2017 21:22
DonJayamanne added a commit that referenced this pull request Dec 14, 2017
* upstream/master:
  Remove jupyter functionality in favor of Jupyter extension (#302)
  Drop Python 2 URLs (#307)
@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
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.

Remove code related to Jupyter as functionality has moved to separate extension
3 participants