Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

@grokys
Copy link
Contributor

@grokys grokys commented Dec 13, 2017

This fixes our lazy loading of assemblies. Previously all assemblies were loaded on VS startup (with Team Explorer and GitHub pane not shown):

image

Following this PR the list now looks like:

image

Note that this only fixes things to the state they were before. ReactiveUI is still used by GitHubConnectSection which in VS2017 is the default view in team explorer, meaning that RxUI gets loaded as soon as Team Explorer loads in VS2017. We should fix this, but it's a different PR I think.

Fixes #1387

Lazily load `IPullRequestSessionManager` in `InlineCommentMarginProvider` to prevent loading the kitchen sink on VS startup.
Was causing Rx to get loaded everywhere.
To prevent `GitHub.App` being loaded when Team Explorer shown.
So that `GitHub.App` is only loaded when actually needed.
@grokys grokys force-pushed the fixes/1387-lazy-load-assemblies branch from 71cb217 to fb88c3e Compare December 13, 2017 17:13
@grokys
Copy link
Contributor Author

grokys commented Dec 14, 2017

This has broken logging in because we're now trying to get the ITwoFactorChallengeHandler service on a background thread.

drguthals
drguthals previously approved these changes Dec 14, 2017
Copy link

@drguthals drguthals left a comment

Choose a reason for hiding this comment

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

lgtm

@meaghanlewis
Copy link
Contributor

LGTM too

jcansdale
jcansdale previously approved these changes Dec 19, 2017
Copy link
Collaborator

@jcansdale jcansdale left a comment

Choose a reason for hiding this comment

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

Looks good. I confirmed the assemblies aren't all loading.

I'll have to remember to lazy load the pull request session manager when displaying the associated PR on the status bar.

jcansdale and others added 2 commits December 19, 2017 14:53
We need to make sure we're on the main thread to get an instance of `ITwoFactorChallengeHandler`. This is nasty and the whole thing needs to be refactored; see #1398.
@grokys grokys dismissed stale reviews from jcansdale and drguthals via 358a653 December 19, 2017 19:35
Copy link
Collaborator

@jcansdale jcansdale left a comment

Choose a reason for hiding this comment

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

I've confirmed that login/2FA is working. 👍 LGTM!

@jcansdale jcansdale merged commit a0d9ad2 into master Dec 20, 2017
@jcansdale jcansdale deleted the fixes/1387-lazy-load-assemblies branch December 20, 2017 10:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All assemblies being loaded on startup.

4 participants