Skip to content
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

Workaround O# path bug #16434

Merged
merged 1 commit into from
Mar 20, 2021
Merged

Workaround O# path bug #16434

merged 1 commit into from
Mar 20, 2021

Conversation

pranavkm
Copy link
Contributor

Workaround for dotnet/aspnetcore#30750

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@pranavkm pranavkm marked this pull request as ready for review March 19, 2021 16:06
@pranavkm pranavkm added the Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch label Mar 19, 2021
@pranavkm pranavkm force-pushed the prkrishn/workaround-omnisharp branch from 06dbc91 to 1d373dc Compare March 19, 2021 17:08
Copy link
Member

@captainsafia captainsafia 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.

Did a spot check on the test failures. The ones I checked were all flakes and not related to the change. 😞

@pranavkm pranavkm force-pushed the prkrishn/workaround-omnisharp branch from 1d373dc to 46db82c Compare March 19, 2021 20:24
@pranavkm pranavkm force-pushed the prkrishn/workaround-omnisharp branch from 46db82c to 4f862db Compare March 19, 2021 23:06
@@ -74,15 +74,18 @@ public void Build_ErrorInGeneratedCode_ReportsMSBuildError()
var projectDirectory = CreateAspNetSdkTestAsset(testAsset);

var filePath = Path.Combine(projectDirectory.Path, "Views", "Home", "Index.cshtml");
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should just make this a relative path and check for the existence of that instead of having the conditional check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was hoping we would undo this PR after we get o# to fix their bug. But using a relative path seems like a good long term fix

@pranavkm pranavkm merged commit 7cf5f24 into main Mar 20, 2021
@pranavkm pranavkm deleted the prkrishn/workaround-omnisharp branch March 20, 2021 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants