Closed aabouzaid closed 4 years ago
this is also a good addition, but there's a little extra work that needs to be done. Variable resolution breaks with this, and many tests fail as a result. For example, properties["ami"]
resolves to ${var.ami
} rather than the intended value, ami-f2d3638a
. See the tests in linter/terraform_test.go
@milldr I've checked it, the values are correct. There is no actual change in the logic.
With a quick look, the tests are fail for 2 reasons:
variable
) and some tests (e.g. TestSingleResourceType)
load file with 3 vars and 1 resources, but assert for 1
, because it's the only type supported at that time ... so there should be some sort of filtering.TestInnerObjects12
) that the tests are tightly coupled with the code ... for example in the old blockTypes, the item resource
comes before data
, but the new blockTypes where blocks are sorted alphabetically, data
comes before resource
, so when the TF file is loaded, they have different index number.I will work on the tests asap.
@milldr I've fixed the tests.
great! Can you please resolve the merge conflicts too?
@milldr sure, I will :+1:
one question here please. do we also need to make the older version v11 compatible with the new changes?
there's no need to make the changes backwards compatible. we're only working to improve v12+ at this point, while supporting v11 as-is
@milldr I'm done :-)
Right now there is only support for the following Terraform blocks "resource", "data", "provider", and "module".
This PR to add support for other blocks "locals", "output", "provider", "terraform", "variable". So they also could be lined.
This fixes issues no.:
163
158
18
Thanks.