Remove 'azurerm_resource_group' & upgrade azurerm 2.0 #35
Conversation
| @@ -47,36 +28,6 @@ module "network" { | |||
| } | |||
| } | |||
|
|
|||
| resource "azurerm_subnet" "subnet" { | |||
There was a problem hiding this comment.
The reason of deleting these examples is? Given the purpose of this README includes showing a runnable example of how to leveraging this module, as it said "This Terraform module deploys a Virtual Network in Azure with a subnet or a set of subnets passed in as input parameters.". So we need to figure out a way to keep the example?
There was a problem hiding this comment.
I find it wield to create a new subnet and a new nsg, while the subnets created in the module can not associate with this nsg. If users want to get their subnets assigned a nsg, they could use another module "vnet"
There was a problem hiding this comment.
I'd like to comment that applying NSG + rules to a VNet + Subnets is a common practice. So it would be very helpful to add an example of how to do that: i.e. Step 1: use this module to create VNet + Subnet. Step 2: Use normal TF resources to create NSG + Rules and apply them to the subnets/one-subnet provisioned in Step 1.
I do not regard being lack of this example is a blocking to the current issue fix, and would suggest you could possibly supplement it in following PRs.
There was a problem hiding this comment.
On fact, "vnet" create "virtual network" and "subnet" itself. In addition, it could assign the subnets the nsg. "network" could not support "nsg" well, so I deleted the example. So I should add one description in README, or just leave it empty?
There was a problem hiding this comment.
I have changed the description to declare it.
There was a problem hiding this comment.
Re "So I should add one description in README, or just leave it empty". No need to describe a non existing thing, if I understood your meanings correctly.
There was a problem hiding this comment.
@yupwei68, I made some comments and pls take a look. thank you.
fix #34