Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

Add variable for tags #36

Merged
merged 1 commit into from
Oct 9, 2023
Merged

Add variable for tags #36

merged 1 commit into from
Oct 9, 2023

Conversation

lonegunmanb
Copy link
Member

Describe your changes

Issue number

#35

Checklist before requesting a review

  • The pr title can be used to describe what this pr did in CHANGELOG.md file
  • I have executed pre-commit on my machine
  • I have passed pr-check on my machine

Thanks for your cooperation!

@lonegunmanb lonegunmanb temporarily deployed to acctests October 8, 2023 08:05 — with GitHub Actions Inactive
@@ -6,10 +6,10 @@ resource "random_integer" "this" {
locals {
account_name = coalesce(var.account_name, "azure-openai-${random_integer.this.result}")
custom_subdomain_name = coalesce(var.custom_subdomain_name, "azure-openai-${random_integer.this.result}")
tags = var.default_tags_enabled ? {
tags = merge(var.default_tags_enabled ? {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@soferreira do we want to give the possibility to add additional tags also when the default tags are enabled ? Or this version of the logic works ?

@JinLee794 feedback is welcome

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah wait, I misunderstood the code.

I understand now correctly that var.tags is always merged in the tags, regarless of the value of var.default_tags_enabled.

Looks good to me !

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks to me!

Choose a reason for hiding this comment

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

Looks good to me!

@zioproto zioproto merged commit abe587b into Azure:main Oct 9, 2023
4 checks passed
zioproto added a commit that referenced this pull request Oct 16, 2023
@zioproto zioproto mentioned this pull request Oct 16, 2023
zioproto added a commit that referenced this pull request Oct 16, 2023
zioproto added a commit that referenced this pull request Oct 16, 2023
zioproto added a commit that referenced this pull request Oct 17, 2023
@zioproto zioproto mentioned this pull request Nov 3, 2023
1 task
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants