Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Implemented state remove command #122

Merged
merged 3 commits into from
Jan 28, 2021

Conversation

aleksanderaleksic
Copy link
Contributor

Implemented the state remove command, with a lot of inspiration from the state move command

@hashicorp-cla
Copy link

hashicorp-cla commented Jan 20, 2021

CLA assistant check
All committers have signed the CLA.

@paultyng
Copy link
Contributor

@aleksanderaleksic this mostly looks good to me, do you think you could also add an e2e test (see the tfexec/internal/e2etest directory). That will at least let it get exercised across multiple versions of TF to ensure its present and works in an expected way.

@aleksanderaleksic
Copy link
Contributor Author

@paultyng Added a test now 👍🏻

Copy link
Contributor

@paultyng paultyng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, would like to have @kmoe give a once over as well though.

@paultyng paultyng requested a review from kmoe January 25, 2021 14:18
@paultyng paultyng added the enhancement New feature or request label Jan 25, 2021
Copy link
Member

@kmoe kmoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, looks good. One comment regarding parameter naming.

tfexec/state_rm.go Outdated Show resolved Hide resolved
@kmoe kmoe merged commit d38206d into hashicorp:master Jan 28, 2021
@aleksanderaleksic aleksanderaleksic deleted the add-remove-state-command branch January 28, 2021 17:14
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants