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

API update for selection change #5516

Merged
merged 2 commits into from
Apr 14, 2021
Merged

API update for selection change #5516

merged 2 commits into from
Apr 14, 2021

Conversation

IanMatthewHuff
Copy link
Member

For #5515

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).

@IanMatthewHuff IanMatthewHuff requested a review from a team as a code owner April 13, 2021 22:51
@@ -267,12 +267,6 @@ export interface NotebookEditor {
*/
readonly document: NotebookDocument;

/**
Copy link
Member Author

Choose a reason for hiding this comment

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

I pulled in just the selection change here. Pulling in the file was bringing in more kernel deprecations that I'd rather deal with the kernel push work.

@DonJayamanne Side note, we seem to have always had three copies of this file. Can we condense down to one? Wasn't sure if there was a reason for the copies, so I didn't do that in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't recall why we had 3, possibly something from core extension. Or maybe when we were using ts-node or the like...
Please do feel free to remove the duplicates.

Copy link
Member Author

Choose a reason for hiding this comment

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

@DonJayamanne sounds good. Will do with kernel API update.

@IanMatthewHuff IanMatthewHuff merged commit f6e1d06 into main Apr 14, 2021
@IanMatthewHuff IanMatthewHuff deleted the dev/ianhu/selectionAPI branch April 14, 2021 01:11
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.

3 participants