Skip to content

Fix generation of SourceKey path in RDG #48017

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 4, 2023
Merged

Fix generation of SourceKey path in RDG #48017

merged 3 commits into from
May 4, 2023

Conversation

@captainsafia
Copy link
Member

@captainsafia captainsafia commented May 2, 2023 •
edited
Loading

Closes #47918.

@ghost ghost added the area-identity Includes: Identity and providers label May 2, 2023
@captainsafia captainsafia force-pushed the safia/rdg-sourcekey-fix branch from a9228fd to ce20fe9 Compare May 2, 2023 15:07
@captainsafia captainsafia force-pushed the safia/rdg-sourcekey-fix branch 2 times, most recently from 22992d0 to 70b10a0 Compare May 3, 2023 15:29
@captainsafia captainsafia force-pushed the safia/rdg-sourcekey-fix branch from 70b10a0 to bf2be28 Compare May 3, 2023 15:49
@captainsafia captainsafia changed the title Reproduce bug in CI Fix generation of SourceKey path in RDG May 3, 2023
@captainsafia captainsafia marked this pull request as ready for review May 3, 2023 17:02
eerhardt
eerhardt approved these changes May 3, 2023
View reviewed changes
Copy link
Member

@eerhardt eerhardt left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I know we will get testing with Microsoft.AspNetCore.Identity, but is there any way to test this directly in RDG? In case Identity decides to not use RDG.

@captainsafia captainsafia force-pushed the safia/rdg-sourcekey-fix branch from f17384e to 96ea99e Compare May 4, 2023 16:55
@captainsafia
Copy link
Member Author

captainsafia commented May 4, 2023

I know we will get testing with Microsoft.AspNetCore.Identity, but is there any way to test this directly in RDG? In case Identity decides to not use RDG.

The route cause of this is the fact that the PathMap wasn't respected in the previous codepath that we had. I tried to see if CompilationOptions or ParseOptions had an option for setting this flag but that doesn't appear to be the case.

I did notice that CompilationOptions allows you to configure the SourceReferenceResolver in tests, but it seems like at that point we're more so testing the behavior of the SourceReferenceResolver than the actual bug.

Outside of those two ideas, I couldn't think of anything else.

@captainsafia captainsafia merged commit 2df5115 into main May 4, 2023
@captainsafia captainsafia deleted the safia/rdg-sourcekey-fix branch May 4, 2023 19:50
@ghost ghost added this to the 8.0-preview5 milestone May 4, 2023
@mthalman mthalman mentioned this pull request May 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@halter73 halter73 halter73 left review comments

@mitchdenny mitchdenny mitchdenny approved these changes

@eerhardt eerhardt eerhardt approved these changes

@Tratcher Tratcher Awaiting requested review from Tratcher

@BrennanConroy BrennanConroy Awaiting requested review from BrennanConroy BrennanConroy is a code owner

@wtgodbe wtgodbe Awaiting requested review from wtgodbe wtgodbe is a code owner

@RikkiGibson RikkiGibson Awaiting requested review from RikkiGibson

Assignees

No one assigned

Labels

area-identity Includes: Identity and providers

Projects

None yet

Milestone

8.0-preview5

Development

Successfully merging this pull request may close these issues.

RDG file name mismatch between SyntaxTree.FilePath and [CallerFilePath] causes runtime KeyNotFoundException

5 participants

@captainsafia @halter73 @mitchdenny @eerhardt