merge tfvars struct into vars struct #75
Conversation
| @@ -257,7 +249,7 @@ func ConvertHCLVariable(tfVar *TFVar) { | |||
|
|
|||
| // GetCreateVariablePayload returns the json needed to make a Post to the | |||
| // Terraform vars api | |||
| func GetCreateVariablePayload(organization, workspaceName string, tfVar TFVar) string { | |||
| func GetCreateVariablePayload(organization, workspaceName string, tfVar Var) string { | |||
| return fmt.Sprintf(` | |||
| { | |||
| "data": { | |||
There was a problem hiding this comment.
I was debating whether or not to update category here to be what was in tfVar, but left it as is for now. I can change it as needed. Also debated making the whole variable payload and using json.Marshal()
There was a problem hiding this comment.
Perhaps an important deciding factor is to make any changes now that would be breaking compatibility. Since this function is exported, any change to the signature or its functionality would be breaking. It might actually make sense to un-export this function now. I'm not sure if it's used anywhere outside this library, though.
There was a problem hiding this comment.
Good points, I can check.
There was a problem hiding this comment.
Also, I might as well fix the deprecated filter.
There was a problem hiding this comment.
Actually. Looking at it further, why not use https://github.com/hashicorp/go-tfe?
[EDIT: I see it has a different license, so yeah, we'd need to factor that in]
[EDIT2: Further reading seems to state that we can leave it as this license (Apache2)
There was a problem hiding this comment.
tfc-ops predates the existence of go-tfe (perhaps only by days) so that's the primary reason why we haven't used go-tfe. The other would be because go-tfe appears to be a thin wrapper around the API, whereas tfc-ops aims to be more opinionated and feature-rich. I'm not sure whether your question was about moving to eliminate tfc-ops as a library, or whether it was to start using go-tfe within this library. I'm open to either one.
There was a problem hiding this comment.
In looking to understand what you meant by "deprecated filter" I noticed that the variables API itself is marked as deprecated.
There was a problem hiding this comment.
I was thinking to use the go-tfe code in this library. It might solve some of the deprecation issues.
| @@ -284,7 +276,7 @@ func GetCreateVariablePayload(organization, workspaceName string, tfVar TFVar) s | |||
|
|
|||
| // GetUpdateVariablePayload returns the json needed to make a Post to the | |||
| // Terraform vars api | |||
| func GetUpdateVariablePayload(organization, workspaceName, variableID string, tfVar TFVar) string { | |||
| func GetUpdateVariablePayload(organization, workspaceName, variableID string, tfVar Var) string { | |||
| return fmt.Sprintf(` | |||
| { | |||
| "data": { | |||
There was a problem hiding this comment.
I was debating whether or not to update category here to be what was in tfVar, but left it as is for now. I can change it as needed. Also debated making the whole variable payload and using json.Marshal()
| @@ -257,7 +249,7 @@ func ConvertHCLVariable(tfVar *TFVar) { | |||
|
|
|||
| // GetCreateVariablePayload returns the json needed to make a Post to the | |||
| // Terraform vars api | |||
| func GetCreateVariablePayload(organization, workspaceName string, tfVar TFVar) string { | |||
| func GetCreateVariablePayload(organization, workspaceName string, tfVar Var) string { | |||
| return fmt.Sprintf(` | |||
| { | |||
| "data": { | |||
There was a problem hiding this comment.
Perhaps an important deciding factor is to make any changes now that would be breaking compatibility. Since this function is exported, any change to the signature or its functionality would be breaking. It might actually make sense to un-export this function now. I'm not sure if it's used anywhere outside this library, though.
| @@ -284,7 +276,7 @@ func GetCreateVariablePayload(organization, workspaceName string, tfVar TFVar) s | |||
|
|
|||
| // GetUpdateVariablePayload returns the json needed to make a Post to the | |||
| // Terraform vars api | |||
| func GetUpdateVariablePayload(organization, workspaceName, variableID string, tfVar TFVar) string { | |||
| func GetUpdateVariablePayload(organization, workspaceName, variableID string, tfVar Var) string { | |||
| return fmt.Sprintf(` | |||
| { | |||
| "data": { | |||
|
@jason-jackson is this ready to merge? |
Removed