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.

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

Tweak Bicep string name. #3331

Merged
merged 2 commits into from
Apr 3, 2024
Merged

Tweak Bicep string name. #3331

merged 2 commits into from
Apr 3, 2024

Conversation

mitchdenny
Copy link
Member

@mitchdenny mitchdenny commented Apr 2, 2024 •
edited by dotnet-policy-service bot
Loading

This PR is a non critical fix for a fix we put into P5. I noticed that filename we generate is resource.bicep not resource.module.bicep. Its a minor cosmetic issue but would be good to fix it for P6.

Closes #3273

This builds upon this PR that went in for P5: #3296

Microsoft Reviewers: Open in CodeFlow

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Apr 2, 2024
@mitchdenny mitchdenny requested a review from eerhardt April 2, 2024 05:47
@mitchdenny mitchdenny self-assigned this Apr 2, 2024
@mitchdenny mitchdenny added this to the preview 6 (Apr) milestone Apr 2, 2024
eerhardt
eerhardt approved these changes Apr 2, 2024
View reviewed changes
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM

@eerhardt
Copy link
Member

eerhardt commented Apr 2, 2024

Looks like a test needs to be updated.

Assert.Equal() Failure
↓ (pos 49)
Expected: ···,\r\n  "path": "templ.bicep",\r\n  "params": {\r\n    "param1": "va···
Actual:   ···,\r\n  "path": "templ.module.bicep",\r\n  "params": {\r\n    "param···
↑ (pos 49)

   at Aspire.Hosting.Tests.Azure.AzureBicepResourceTests.AssertManifestLayout() in /_/tests/Aspire.Hosting.Tests/Azure/AzureBicepResourceTests.cs:line 158
--- End of stack trace from previous location ---

@mitchdenny mitchdenny merged commit f077b95 into main Apr 3, 2024
8 checks passed
@mitchdenny mitchdenny deleted the mitchdenny/tweak-bicep-string-name branch April 3, 2024 05:23
@mitchdenny
Copy link
Member Author

/backport to release/8.0

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 3, 2024

Started backporting to release/8.0: https://github.com/dotnet/aspire/actions/runs/8533667270

@github-actions github-actions bot mentioned this pull request Apr 3, 2024
Merged
radical pushed a commit to radical/aspire that referenced this pull request Apr 3, 2024
@mitchdenny @radical
Tweak Bicep string name. (dotnet#3331)
b940c0c 
* Tweak Bicep string name.

* Fix up test.
@github-actions github-actions bot locked and limited conversation to collaborators May 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eerhardt eerhardt eerhardt approved these changes

Assignees

@mitchdenny mitchdenny

Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
Milestone
preview 6 (Apr)
Development

Successfully merging this pull request may close these issues.

AddBicepTemplateString should output a resourceName.module.bicep instead of a random path.
2 participants
@mitchdenny @eerhardt