Skip to content

Conversation

@sharwell
Copy link
Contributor

Reverts #67030 pending design review.

@sharwell sharwell requested a review from a team as a code owner February 24, 2023 15:55
@sharwell sharwell enabled auto-merge February 24, 2023 15:59
@CyrusNajmabadi
Copy link
Member

Reasoning for disabling were given in the original PR. Even if this is useful (very questionable), it has to warrant its cost. RIght now it does an enormous amount of work (like running source generators) just to show a potential squiggle for a few seconds. We can reenable this once we show we can do so in a way that doesn't cost as much.

This feature is also not going to be there once we switch onto pull-diagnostics (expected to go into dogfooding next week), and is not supported as part of LSP. So any time this would be back on would be short lived. Given that, and the poor perf, turnign off to give people direct perf benefits for actually important scenarios (lightbulb) is more critical.

@sharwell
Copy link
Contributor Author

This still needed to go to a team design review in advance of the merge.

@sharwell sharwell merged commit 3f192dc into main Feb 24, 2023
@ghost ghost added this to the Next milestone Feb 24, 2023
@CyrusNajmabadi CyrusNajmabadi deleted the revert-67030-disablePreviewDiagnostics branch February 24, 2023 18:09
@RikkiGibson RikkiGibson modified the milestones: Next, 17.6 P2 Mar 2, 2023
@mavasani mavasani added the Performance-Scenario-Diagnostics This issue affects diagnostics computation performance for lightbulb, background analysis, tagger. label May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Performance-Scenario-Diagnostics This issue affects diagnostics computation performance for lightbulb, background analysis, tagger.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants