Acl block to file #253
Acl block to file #253
Conversation
There was a problem hiding this comment.
LGTM! A couple comments that would make it more testable and align with logging endpoint structure, not blocking though.
Also, looks like CI is failing just due to formatting (see this). You should be able to just run make fmt to fix.
| // Delete removed ACL configurations | ||
| for _, vRaw := range remove { | ||
| val := vRaw.(map[string]interface{}) | ||
| opts := gofastly.DeleteACLInput{ |
There was a problem hiding this comment.
Not blocking, but in the open logging PRs, we pulled the building of Delete*Input into a separate function (see this).
| } | ||
|
|
||
| log.Printf("[DEBUG] Fastly ACL removal opts: %#v", opts) | ||
| err := conn.DeleteACL(&opts) |
There was a problem hiding this comment.
Not blocking, similar to the above comment, we pulled this block out into its own function (see this).
| _, err := conn.CreateACL(&opts) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Not blocking, similar to the above comments (see this).
Pulling it out separately would make it easier to inject dependencies in tests.
There was a problem hiding this comment.
Sorry, this wouldn't actually help with DI in tests. But arguably improves readability and could potentially help in future refactoring.
| opts := gofastly.CreateACLInput{ | ||
| Service: d.Id(), | ||
| Version: latestVersion, | ||
| Name: val["name"].(string), | ||
| } |
There was a problem hiding this comment.
Not blocking, similar to the above comments (see this).
Pulling it out separately would make it easier to inject dependencies in tests.
There was a problem hiding this comment.
Sorry, this wouldn't actually help with DI in tests. But arguably improves readability and could potentially help in future refactoring.
Factored out the ACL block code from fastly/resource_fastly_service_v1.go into it's own block file, as POC for factoring out all block code.