Skip to content

Enable live share support#4645

Merged
rchiodo merged 15 commits into
masterfrom
rchiodo/live_share_testing
Mar 6, 2019
Merged

Enable live share support#4645
rchiodo merged 15 commits into
masterfrom
rchiodo/live_share_testing

Conversation

@rchiodo

@rchiodo rchiodo commented Mar 5, 2019

Copy link
Copy Markdown

For #4521, #4529

Finish enabling live share support and add tests for liveshare and color parsing.
Fix issue with liveshare not bundling correctly.

  • 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!)
  • 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)
  • The wiki is updated with any design decisions/details.

@rchiodo rchiodo self-assigned this Mar 5, 2019

export const IJupyterExecution = Symbol('IJupyterExecution');
export interface IJupyterExecution extends IAsyncDisposable {
sessionChanged: Event<void> ;

@rchiodo rchiodo Mar 5, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sessionChanged [](start = 4, length = 14)

This was added so that if a history window was host and then suddenly switched to guest, it would dispose of the server in use #ByDesign

@codecov

codecov Bot commented Mar 5, 2019

Copy link
Copy Markdown

Codecov Report

Merging #4645 into master will increase coverage by 1%.
The diff coverage is 46%.

@@           Coverage Diff           @@
##           master   #4645    +/-   ##
=======================================
+ Coverage      77%     77%    +1%     
=======================================
  Files         445     446     +1     
  Lines       21074   21126    +52     
  Branches     3450    3452     +2     
=======================================
+ Hits        16118   16232   +114     
+ Misses       4952    4890    -62     
  Partials        4       4
Flag Coverage Δ
#Linux 66% <36%> (+1%) ⬆️
#Windows 66% <36%> (+1%) ⬆️
#macOS 66% <36%> (+1%) ⬆️

Comment thread build/webpack/common.js
return files.map(filePath => `./${filePath.slice(0, -3)}`);
}
exports.getListOfExistingModulesInOutDir = getListOfExistingModulesInOutDir;
// Copyright (c) Microsoft Corporation. All rights reserved.

@IanMatthewHuff IanMatthewHuff Mar 5, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code flow is not showing a change in this file. What changed? #ByDesign

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah there's a bunch like this. Or there's about to be. I think it's just line endings. Git will skip these on the real commit.


In reply to: 262688394 [](ancestors = 262688394)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sorry the 'about to be' is me ingesting a new liveshare npm module. They broke our install again with a requirement we don't minify their js.


In reply to: 262689934 [](ancestors = 262689934,262688394)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

And that means I have to rework the 'fix' for release mode again.


In reply to: 262690183 [](ancestors = 262690183,262689934,262688394)

// Only do this once as it crashes if we ask more than once
if (!this.vscodeApi &&
// tslint:disable-next-line:no-typeof-undefined
typeof acquireVsCodeApi !== 'undefined') {

@IanMatthewHuff IanMatthewHuff Mar 5, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

// Comment
// tslint:
if (!this.vscodeApi && typeof acquireVsCodeApi !=== 'undefined') {

Would read better unless there was a reason to break it up. #Resolved

runTest('Tomorrow Night Blue', true, true);

// One test to make sure unknown themes don't return a value.
runTest('Knight Rider', true, false);

@IanMatthewHuff IanMatthewHuff Mar 5, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm going to add a Knight Rider theme now... #WontFix

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Only if it has a hasselhoff scope.


In reply to: 262697179 [](ancestors = 262697179)

container.wrapper = mounted;

// We can remove the global api and event listener now.
delete (global as any)['ascquireVsCodeApi'];

@IanMatthewHuff IanMatthewHuff Mar 5, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

misspell function name. #Resolved

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

whoops. will fix


In reply to: 262715314 [](ancestors = 262715314)

assert.ok(both, 'Expected both guest and host to be used');
await codeWatcher.runAllCells();
});
verifyHtmlOnCell(wrapper, '<span>1</span>', CellPosition.Last);

@IanMatthewHuff IanMatthewHuff Mar 5, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

verify hostContainer.wrapper here as well? #Resolved

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

good idea


In reply to: 262717056 [](ancestors = 262717056)

@IanMatthewHuff IanMatthewHuff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

rchiodo added 2 commits March 5, 2019 15:32
changes from vsls sending out onsessionchanged more often
@rchiodo rchiodo merged commit 3d9c962 into master Mar 6, 2019
@rchiodo rchiodo deleted the rchiodo/live_share_testing branch March 6, 2019 01:41
@lock lock Bot locked as resolved and limited conversation to collaborators Jul 30, 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.

2 participants