-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Remove PortablePdbReader except trace cleanup #19957
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
Conversation
Thanks for the PR, @RozzaysRed. |
Actually, the related issue is |
Does this affect the error page in any way? |
@davidfowl likely not - we added this type because .NET Fx was unable to natively read portable pdbs. https://github.com/dotnet/aspnetcore/blob/master/src/Mvc/test/Mvc.FunctionalTests/ErrorPageTests.cs has some tests that should cover end-to-end scenarios to an extent. |
@pranavkm Kewl. @RozzaysRed can you show a screenshot of the DeveloperExceptionPage after this change? |
@davidfowl sure as soon I figure out how to get this page, I will take the requested screenshot. I'm assuming its a dummy project and enabling that middleware? Pardon me for asking but just want to make sure I get you exactly what you are looking for. |
@RozzaysRed you can run the https://github.com/dotnet/aspnetcore/blob/master/src/Mvc/samples/MvcSandbox and include screenshots for the result when an exception is thrown from
The sample is already set up to include the developer error page middleware |
Would you mind including screenshot of an exception thrown from inside a view? We've had issues with it in the past, so it would be good to confirm that looks correct. |
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.
Looks great!
src/Shared/test/Shared.Tests/Microsoft.AspNetCore.Shared.Tests.csproj
Outdated
Show resolved
Hide resolved
@pranavkm sorry about that, I thought I had pasted the file that had both. Here is the view: |
Thanks! |
Undo changes to the csproj file
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.
Thanks!
Summary of the changes (Less than 80 chars)
Addresses #19441