-
Notifications
You must be signed in to change notification settings - Fork 357
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
awsrules: add tags package with generator #803
Conversation
@wata727 Wanted to check in on this one before I go any further now that I have something working. If you're interested, I can expand this PR to similarly relocate the other |
Looks great. The reason for defining another dependency under It would be nice if the dependencies were integrated and everything was generated by |
//go:generate mockgen -destination aws_elbv2_mock.go -package client github.com/aws/aws-sdk-go/service/elbv2/elbv2iface ELBV2API | ||
//go:generate mockgen -destination aws_iam_mock.go -package client github.com/aws/aws-sdk-go/service/iam/iamiface IAMAPI | ||
//go:generate mockgen -destination aws_rds_mock.go -package client github.com/aws/aws-sdk-go/service/rds/rdsiface RDSAPI | ||
//go:generate mockgen -destination aws_ecs_mock.go -package client github.com/aws/aws-sdk-go/service/ecs/ecsiface ECSAPI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow! It was a big problem for me that I had to run go mod vendor
for mockgen. Great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Awesome! I'll make some time to do the same with the other generators. Figured it was a dependency issue. |
github.com/jessevdk/go-flags v1.4.0 | ||
github.com/mattn/go-colorable v0.1.6 | ||
github.com/mitchellh/go-homedir v1.1.0 | ||
github.com/sourcegraph/go-lsp v0.0.0-20181119182933-0c7d621186c1 | ||
github.com/sourcegraph/jsonrpc2 v0.0.0-20190106185902-35a74f039c6a | ||
github.com/spf13/afero v1.2.2 | ||
github.com/terraform-linters/tflint-plugin-sdk v0.1.2-0.20200615160547-c1d3caf80fe0 | ||
github.com/terraform-providers/terraform-provider-aws v2.65.0+incompatible // indirect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bendrucker I noticed this line is removed when run go mod tidy
because the code required this dependency is ignored on the build phase. This seems a little inconvenient. Any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently this is a special case of +build ignore
and any other constraint name will work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be resolved by #809
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
This PR adds a generator as a child package in a similar style to terraform-provider-aws:
https://github.com/terraform-providers/terraform-provider-aws/tree/master/aws/internal/keyvaluetags
In responding to https://github.com/terraform-linters/tflint/issues/788 I realized that the working in aws_missing_tags is not very adapatable to other rules. It templates an entire file with several hundred lines to substitute one list of resources types.
Instead, I've created a package to handle tags and a sub-package to generate a file that exports the list of tagged resource types. From there I plan to pull out the tag evaluation functionality into the
tags
package.This may also pave the way for breaking apart the
tools
packages, moving generator packages to where they are used, and invoking them via//go:generate
comments.Wanted to open this early to check on interesting in breaking apart
tools
and whether there's a reason having separate dependencies is important/required.