-
Notifications
You must be signed in to change notification settings - Fork 679
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
Update Razor to include more C# actions #3524
Update Razor to include more C# actions #3524
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@ryanbrandenburg the build failure looks legit. Since we removed a good amount of telemetry this test is now failing: https://github.com/OmniSharp/omnisharp-vscode/blob/2edb8414981d49185ce9fe783187008ab343678b/test/razorTests/razorTelemetry.integration.test.ts#L46 Sadly there isn't a Razor telemetry event that O# can capture during their tests because the suite teardown/restore is when all of our existing telemetry events fire and during test time you can't control exactly when that happens. |
Codecov Report
@@ Coverage Diff @@
## master #3524 +/- ##
======================================
Coverage 89.8% 89.8%
======================================
Files 59 59
Lines 1589 1589
Branches 89 89
======================================
Hits 1427 1427
Misses 151 151
Partials 11 11
Continue to review full report at Codecov.
|
09030fa
to
1305d32
Compare
@JoeRobich FYI the telemetry test is being removed because we no longer publish that telemetry event. That seemed to be the only test in it's suite so I removed the suite and its infrastructure too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be great!
CC @NTaylorMullen and @ajaybhargavb.