-
Notifications
You must be signed in to change notification settings - Fork 138
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
Feature: Add flag for install location (optional) #309
Conversation
This would be very useful for CI/CD cases where $HOME/ is not a cache-able path. |
For CI/CD is what I wrote this for. It turns out we found a workaround for the issue we were having so I didn't need this in the end. I created a test binary and published it here if you want it but I am not using it myself, I fell back to the original version by warrensbox. I think its just linux_amd64 binary only though: |
We can revisit this after release v1.10 |
8ec9f51
to
48719a5
Compare
Updated with latest master changes. I have moved away from using tfswitch and tgswitch now. But I wanted to attempt to get this PR updated to as best I can. Since I haven't touched this project in over 9 months, it took a while. I "think" it now works. I tested locally quickly and the If this is still wanted please ping me if you need anything. I will leave this PR in draft as I cannot fully test this locally. Feel free to take the code and create a new branch/PR if that is easier. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
26738f0
to
7272162
Compare
Resolved all comments. If you ping me when #356 is merged. I will update this again, and re-test it. |
Thank you.
We'll try out best 🤝 |
@ArronaxKP Pinging you |
eabfda1
to
05ad223
Compare
As promised the PR updated with latest master. Includes the origina Spotted some other warnings in other files I thought I would fix.
|
6083aa2
to
bae464c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with minor note below
@warrensbox @MatrixCrawler @crablab @MatthewJohn @jukie Please review. |
Add the ability to pass -i or --install to change the default install location for the Terraform binaries
bae464c
to
b460ffc
Compare
Updated with latest master. My Github actions is passing as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Will look this evening/tomorrow morning. |
I'm giving this my "LGTM". Please merge once you get this reviewed and if you approve when you have a chance later. Thanks. |
This change adds a new flag
-i
or--install
that will allow you to set where the Terraform binary is downloaded/installed into. Useful for edge cases where you need to share commonly installed versions of Terraform.This is in draft as it is built upon this PRs that should be merged first. To save headaches with merge conflicts this PR already includes that commit.
Once the above PR has been merged ping me and I can update this branch with latest master.
I have created a similar PR for tgswitch here - warrensbox/tgswitch#140