Add ability to specify path to TF binary #109
Conversation
|
@radeksimko made the suggested adjustments, also added some testing of the path string to make sure its a bit more valid as discussed. I cleaned up a few of the other |
There was a problem hiding this comment.
As for the ui.Error calls - this was actually intentional, as I thought errors should have a blank line below them, to express the visual importance of the message. But I do see how it may look awkward with all these explicit \ns everywhere, so I don't have strong feelings about it.
Thank you for adding the validation logic 👍
I left just one comment, but it's not a blocker.
| if stat.IsDir() { | ||
| c.Ui.Error(fmt.Sprintf("Expected a Terraform binary, got a directory: %q", path)) | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
Total nitpick, but I assume the above lines could make a validateTfExecPath(string) error function.
|
The confusing part was they weren't on all errors just some errors, but if its the how it should be presented you could already create a fancy UI implementation that adds the separation. I was hoping to maybe revisit/refactor this after the release anyway, clean up the hard coded exit codes and flag validation a bit. I'll just write an issue to do so if thats ok with you? |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
ghost
locked and limited conversation to collaborators
No description provided.