Skip to content
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

[WIP] replace "state name" with "workspace" #24

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mcanevet
Copy link
Contributor

@mcanevet mcanevet commented Jun 15, 2019

Will fix #18. I think we should clarify this before going further because it will probably break the API (at least the internal one).
I still have a lot of questions for you @cryptobioz and @raphink so I open this PR so that we can discuss about that in comments.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@mcanevet mcanevet requested review from raphink and cryptobioz June 15, 2019 06:45
@mcanevet
Copy link
Contributor Author

mcanevet commented Jun 15, 2019

In pkg/client/client.go and internal/storage/storage.go, does ListStates retrieve all states or workspaces? If it's workspaces, then we should rename it to ListWorkspaces.

@mcanevet
Copy link
Contributor Author

mcanevet commented Jun 15, 2019

In internal/storage/storage.go, should the Name attribute of the State structure be Workspace?
Basically, shouldn't we have a Workspace structure with a States []*terraform.State and a Plans []*terraform.Plan attributes? Because we plan to store plans using Terraform v0.12's new show <plan> json output. Maybe it just a matter of renaming the StateCollection structure?

@mcanevet
Copy link
Contributor Author

mcanevet commented Jun 15, 2019

Hmmm... Actually, if we add a Workspace structure, we could reuse the State structure defined in github.com/hashicorp/terraform/terraform/state.go instead of duplicating most of its code. What do you think about that? Same with the Plan structure.

@mcanevet
Copy link
Contributor Author

mcanevet commented Jun 15, 2019

The more I read at the code, the more I think this is a huge change... But I still think we should do it.
I guess we should add this structure and refactor a lot of things:

type Workspace struct {
    LastModified time.Time `json:"last_modified"`
    Name         string    `json:"name"`
 
    // Keep Lock info
    Locked   bool     `json:"locked"`
    LockInfo LockInfo `json:"lock"`
 
    States []*terraform.State
    Plans  []*terraform.Plan
}

@mcanevet
Copy link
Contributor Author

I added a new ListWorkspaces function. Maybe we should use this one instead of ListStates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use "workspace" instead of "project" or "state name"
1 participant