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

Symbolic names: Generate dependsOn for "existing" resources #11478

Merged
merged 2 commits into from
Aug 29, 2023

Conversation

anthony-c-martin
Copy link
Member

@anthony-c-martin anthony-c-martin commented Aug 11, 2023

Closes #10343

@github-actions
Copy link
Contributor

Test Results (linux-x64)

       33 files  ±0         33 suites  ±0   35m 12s ⏱️ - 4m 24s
10 398 tests +1  10 362 ✔️ +1  36 💤 ±0  0 ±0 
12 615 runs  +1  12 579 ✔️ +1  36 💤 ±0  0 ±0 

Results for commit f3c13f5. ± Comparison against base commit 6c3a71d.

@github-actions
Copy link
Contributor

Test Results (linux-musl-x64)

       33 files  ±0         33 suites  ±0   37m 6s ⏱️ + 1m 24s
10 398 tests +1  10 362 ✔️ +1  36 💤 ±0  0 ±0 
12 615 runs  +1  12 579 ✔️ +1  36 💤 ±0  0 ±0 

Results for commit f3c13f5. ± Comparison against base commit 6c3a71d.

@github-actions
Copy link
Contributor

Test Results (win-x64)

       33 files  ±0         33 suites  ±0   32m 52s ⏱️ - 3m 48s
10 410 tests +1  10 374 ✔️ +1  36 💤 ±0  0 ±0 
12 626 runs  +1  12 590 ✔️ +1  36 💤 ±0  0 ±0 

Results for commit f3c13f5. ± Comparison against base commit 6c3a71d.

@github-actions
Copy link
Contributor

Test Results (osx-x64)

       33 files  ±0         33 suites  ±0   1h 39m 3s ⏱️ + 11m 9s
10 402 tests +1  10 366 ✔️ +1  36 💤 ±0  0 ±0 
12 619 runs  +1  12 583 ✔️ +1  36 💤 ±0  0 ±0 

Results for commit f3c13f5. ± Comparison against base commit 6c3a71d.

Copy link
Contributor

@jeskew jeskew left a comment

Choose a reason for hiding this comment

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

I believe this isn't strictly necessary since a reference function that refers to a resource by symbolic name and does not specify an API version creates an implicit dependency in ARM, but this does give us the flexibility to start using API versions in symbolic name reference functions if so desired.

@anthony-c-martin anthony-c-martin merged commit feb4930 into main Aug 29, 2023
52 checks passed
@anthony-c-martin anthony-c-martin deleted the ant/issue10343 branch August 29, 2023 15:03
@anthony-c-martin
Copy link
Member Author

I believe this isn't strictly necessary since a reference function that refers to a resource by symbolic name and does not specify an API version creates an implicit dependency in ARM, but this does give us the flexibility to start using API versions in symbolic name reference functions if so desired.

That's fair. In this case it wasn't a conscious change - I had intended to do this, but the code I'd written didn't behave in the way I intended it to (even if it happened to work!). So here I just wanted to bring the actual behavior in-line with my own mental model.

We could definitely revise the approach in the future if we feel like dependsOn isn't necessary at all - I'd just like that to be a conscious decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Symbolic name template codegen dependsOn statements not generated correctly for "existing" resources
2 participants