-
Notifications
You must be signed in to change notification settings - Fork 533
Allow consumers to point git binary via env var #416
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
Conversation
lib/git/config.rb
Outdated
@@ -10,7 +10,7 @@ def initialize | |||
end | |||
|
|||
def binary_path | |||
@binary_path || 'git' | |||
@binary_path || ENV['GIT_BIN'] || 'git' |
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.
How about GIT_PATH
instead? Or GIT_BINARY
? (I would prefer the first option, but interested in hearing your opinions also.)
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.
In my opinion, pointing to the actual binary is the best way to give users the flexibility that they need. Though I can see the confusion since the method name is binary_path
. I could rename the function if that makes it more clear since it is not returning the path of the binary but the binary itself.
What do you think @perlun?
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.
@afiune Thanks. I was just thinking about changing the environment variable name, sorry for not being so clear here. 👍
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.
Looked at your change now. I think GIT_PATH
would be the clearer way. Sorry for being slightly picky, but please change the env variable to that and I'll be happy to get this merged ASAP.
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.
Sounds good. I’ll make the change, update the tests and documentation. 💯
I just renamed the environment variable as you suggested, I can change the behavior to provide the path is this is what you think is best for the users. Open to suggestions. |
@perlun Ready to merge! 💯 |
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.
Looks good! 👍 Thanks. Please just get the commits squashed into one (and edit the commit message to be what you want it to be in the repo) and I'll get this merged.
By adding a new environment variable called `GIT_PATH` we can allow consumers, that is, a user of a gem that the gem itself uses the git gem, to customize the location of the git binary. Example: Having a gem called `git-tool` that uses this gem `git`, if I, as a user wants to modify the git bin location, I could do: ``` GIT_PATH=/foo/bin git-tool bar ``` Signed-off-by: Salim Afiune <afiune@chef.io>
cab28f2
to
4988861
Compare
@perlun done done done!! |
@perlun ping 😅 |
Looks good @afiune! Sorry for the delays on this. Merging now. |
By adding a new environment variable called `GIT_PATH` we can allow consumers, that is, a user of a gem which itself uses the git gem, to customize the location of the git binary. Example: Having a gem called `git-tool` that uses this gem `git`, if I, as a user wants to modify the git bin location, I could do: ``` GIT_PATH=/foo/bin git-tool bar ``` Signed-off-by: Salim Afiune <afiune@chef.io>Signed-off-by: Agora Security <github@agora-security.com>
Your checklist for this pull request
🚨Please review the guidelines for contributing to this repository.
Description
By adding a new environment variable called
GIT_PATH
we can allowconsumers, that is, a user of a gem that the gem itself uses the git gem,
to customize the location of the git binary.
Example: Having a gem called
git-tool
that uses this gemgit
, if I,as a user wants to modify the git bin location, I could do:
Signed-off-by: Salim Afiune afiune@chef.io