Skip to content

Re-enable find-references progress.#42430

Merged
CyrusNajmabadi merged 1 commit intodotnet:release/dev16.7-preview1from
CyrusNajmabadi:findRefsProgress
Apr 4, 2020
Merged

Re-enable find-references progress.#42430
CyrusNajmabadi merged 1 commit intodotnet:release/dev16.7-preview1from
CyrusNajmabadi:findRefsProgress

Conversation

@CyrusNajmabadi
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi commented Mar 14, 2020 •
edited
Loading

Fixes #15063

Have manually tested and profiled this to make sure there is no perf regression like last time.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner March 14, 2020 23:14
CyrusNajmabadi
CyrusNajmabadi commented Mar 14, 2020
View reviewed changes
src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncBatchingWorkQueue.cs Outdated
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Mar 14, 2020

Choose a reason for hiding this comment

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

Extracted from #42346

Copy link
Contributor

@mavasani mavasani Mar 15, 2020

Choose a reason for hiding this comment

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

Hmm I am unable find this file or functionality in that PR - is this shared utility between designer attribute incremental analyzer and FAR now?

Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Mar 16, 2020

Choose a reason for hiding this comment

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

in that PR it's embedded in a type. i'm pulling it out so it can be shared with multiple services. it's also pulled out in #42383

mavasani
mavasani reviewed Mar 15, 2020
View reviewed changes
.../Core/Def/Implementation/FindReferences/Contexts/AbstractTableDataSourceFindUsagesContext.cs Outdated
Copy link
Contributor

@mavasani mavasani Mar 15, 2020

Choose a reason for hiding this comment

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

Do we want to consider flighting this change, say just for internal users, instead of turning it on for everyone by default? @jinujoseph how do we push flighting changes nowadays?

mavasani
mavasani reviewed Mar 15, 2020
View reviewed changes
Copy link
Contributor

@mavasani mavasani left a comment

Choose a reason for hiding this comment

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

Changes LGTM, I have more of a logistical question on whether we should flight this change first, given the experience in this space has regressed in the past. I would let @jinujoseph sign off if we are fine in not flighting.

@jinujoseph
Copy link
Contributor

jinujoseph commented Mar 16, 2020

Thanks @mavasani, i agree we should flight this experience
@CyrusNajmabadi lets chat about this tomorrow

@CyrusNajmabadi
Copy link
Contributor Author

CyrusNajmabadi commented Mar 16, 2020

@CyrusNajmabadi lets chat about this tomorrow

Sure! :)

@CyrusNajmabadi
Copy link
Contributor Author

CyrusNajmabadi commented Mar 20, 2020

Ok. We've kicked off a CI build for this: https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_build/results?buildId=3570956

@ryzngard and @genlu are helping me with this.

@CyrusNajmabadi
Copy link
Contributor Author

CyrusNajmabadi commented Mar 26, 2020

@genlu was everything ok here?

@CyrusNajmabadi CyrusNajmabadi changed the base branch from master to release/dev16.7-preview1 April 4, 2020 00:32
@CyrusNajmabadi CyrusNajmabadi changed the base branch from release/dev16.7-preview1 to master April 4, 2020 00:32
@CyrusNajmabadi CyrusNajmabadi requested a review from mavasani April 4, 2020 00:32
@CyrusNajmabadi CyrusNajmabadi force-pushed the findRefsProgress branch 3 times, most recently from 2750659 to 084e2d6 Compare April 4, 2020 01:59
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner April 4, 2020 01:59
@CyrusNajmabadi CyrusNajmabadi requested a review from a team April 4, 2020 01:59
@CyrusNajmabadi CyrusNajmabadi changed the base branch from master to release/dev16.7-preview1 April 4, 2020 01:59
@CyrusNajmabadi CyrusNajmabadi requested a review from genlu April 4, 2020 01:59
ghost
ghost approved these changes Apr 4, 2020
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@CyrusNajmabadi CyrusNajmabadi merged commit 7e5ac01 into dotnet:release/dev16.7-preview1 Apr 4, 2020
@CyrusNajmabadi CyrusNajmabadi deleted the findRefsProgress branch April 4, 2020 06:42
@jinujoseph jinujoseph added this to the 16.7.P1 milestone Apr 5, 2020
@Youssef1313 Youssef1313 mentioned this pull request Apr 2, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@genlu genlu Awaiting requested review from genlu

3 more reviewers

@sharwell sharwell sharwell left review comments

@mavasani mavasani mavasani approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jinujoseph jinujoseph

Labels

Area-IDE auto-merge

Projects

None yet

Milestone

16.7.P1

Development

Successfully merging this pull request may close these issues.

Remove 'catch' from UpdateProgress added in https://github.com/dotnet/roslyn/pull/15037

5 participants

@CyrusNajmabadi @jinujoseph @genlu @sharwell @mavasani