-
Notifications
You must be signed in to change notification settings - Fork 798
Making the scope property as fully qualified resource Id #18165
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
base: main
Are you sure you want to change the base?
Conversation
Test this change out locally with the following install scripts (Action run 18287856178) VSCode
Azure CLI
|
Dotnet Test Results 96 files - 48 96 suites - 48 42m 8s ⏱️ - 28m 23s Results for commit a22ca09. ± Comparison against base commit 066c054. This pull request removes 1924 and adds 661 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
{ | ||
// emit the resource id of the resource being extended | ||
var indexContext = TryGetReplacementContext(scopeResource, resource.ScopeData.IndexExpression, resource.BodySyntax); | ||
expressionEmitter.EmitProperty("scope", () => expressionEmitter.EmitUnqualifiedResourceId(scopeResource, indexContext)); |
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.
Is EmitUnqualifiedResourceId
still used anywhere? If not, can you remove it?
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.
I have removed this reference
public void EmitFullyQualifiedResourceId(DeclaredResourceMetadata resource, IndexReplacementContext? indexContext) | ||
{ | ||
var converterForContext = converter.GetConverter(indexContext); | ||
|
||
var fullyQualifiedResourceId = converterForContext.GetFullyQualifiedResourceId(resource); | ||
var serialized = ExpressionSerializer.SerializeExpression(fullyQualifiedResourceId); | ||
|
||
writer.WriteValue(serialized); | ||
} |
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.
Unless I'm mistaken, the implementation looks identical to the existing function EmitResourceIdReference
- can we commonize rather than creating a new one?
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, for pointing this out. I have renamed the existing one, just to be more explicit.
Description
The PR, makes scope property fully qualified resource Id.
Checklist
Microsoft Reviewers: Open in CodeFlow