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.

Sign up for GitHub

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

Jump to bottom

Switch to hc-install from tfinstall #737

Merged
merged 8 commits into from
Jan 11, 2022
Merged

Switch to hc-install from tfinstall #737

merged 8 commits into from
Jan 11, 2022

Conversation

rtim75
Copy link
Contributor

@rtim75 rtim75 commented Dec 7, 2021 •
edited
Loading

Closes #711

  • replaced all tfinstall usage with hc-install which first looks for tf in filesystem and if didn't find, installs specified/latest version
  • in exec moved tf version into function arguments

@hashicorp-cla
Copy link

hashicorp-cla commented Dec 7, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@radeksimko radeksimko added enhancement New feature or request technical-debt labels Dec 7, 2021
radeksimko
radeksimko reviewed Dec 13, 2021
View reviewed changes
Copy link
Member

@radeksimko radeksimko 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 for the PR!

I left you some suggestions in-line. I'm happy to discuss any point further.

internal/langserver/handlers/complete_test.go Show resolved Hide resolved
internal/terraform/exec/exec_test.go Outdated Show resolved Hide resolved
internal/langserver/handlers/complete_test.go Outdated Show resolved Hide resolved
internal/langserver/handlers/complete_test.go Show resolved Hide resolved
internal/schemas/gen/gen.go Show resolved Hide resolved
internal/schemas/gen/gen.go Show resolved Hide resolved
internal/terraform/exec/exec_test.go Outdated Show resolved Hide resolved
internal/terraform/exec/exec_test.go Outdated Show resolved Hide resolved
radeksimko
radeksimko reviewed Jan 4, 2022
View reviewed changes
Copy link
Member

@radeksimko radeksimko 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 for making the changes. I'm happy to merge once the last two comments about cleanup are resolved.

internal/langserver/handlers/complete_test.go Show resolved Hide resolved
internal/terraform/exec/exec_test.go Outdated Show resolved Hide resolved
@rtim75
Copy link
Contributor Author

rtim75 commented Jan 5, 2022

Thanks @radeksimko
I've updated all the files to have explicit cleanups for install dirs as well as for installers.

@rtim75 rtim75 requested a review from radeksimko January 5, 2022 11:39
@rtim75
Copy link
Contributor Author

rtim75 commented Jan 11, 2022

I'm looking into the failing build

@rtim75
Copy link
Contributor Author

rtim75 commented Jan 11, 2022 •
edited
Loading

@radeksimko should the hpegl be excluded as well? It appears to be the same thing as #692

@radeksimko
Copy link
Member

I have just addressed a few conflicts (mainly in go.mod & go.sum) and one remaining point about where Terraform gets installed, so we can get this merged - I hope you don't mind. Your original authorship remains intact.

Thank you for the contribution!

@rtim75
Copy link
Contributor Author

rtim75 commented Jan 11, 2022 •
edited
Loading

I hope you don't mind

Not at all 😄 Thanks)

@radeksimko radeksimko merged commit 7dd9d89 into hashicorp:main Jan 11, 2022
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@radeksimko radeksimko radeksimko approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request technical-debt
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Switch to hc-install from tfinstall
3 participants
@rtim75 @hashicorp-cla @radeksimko