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.

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

Allow git-secret to run from any directory in the work tree #56

Merged
merged 1 commit into from
Dec 9, 2016
Merged

Allow git-secret to run from any directory in the work tree #56

merged 1 commit into from
Dec 9, 2016

Conversation

timchurchard
Copy link
Contributor

Hi, this allows git-secret to work from any working directory in the git repository. The .gitsecret directory remains in the working directory allowing users to have more than one .gitsecret in the repo.

This will result in a small change of behaviour. If a user runs git-secret from the non-root of the repo' instead of exiting with 'not a repo' error it will exit with '.gitsecret directory not found'.

@sobolevn
Copy link
Owner

sobolevn commented Dec 9, 2016

Thanks! That's a good idea. But I have some minor comments.

@@ -4,9 +4,12 @@ set -e

function _check_setup {
# Checking git and secret-plugin setup:
if [[ ! -d ".git" ]] || [[ ! -d ".git/hooks" ]]; then
set +e
Copy link
Owner

Choose a reason for hiding this comment

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

We use set -e in all our code, I guess there's no reason to break the rule here.

@@ -4,9 +4,12 @@ set -e

function _check_setup {
# Checking git and secret-plugin setup:
if [[ ! -d ".git" ]] || [[ ! -d ".git/hooks" ]]; then
set +e
git rev-parse --is-inside-work-tree >/dev/null 2>&1
Copy link
Owner

Choose a reason for hiding this comment

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

It is better to move this function into utils. I guess it could be named _is_inside_git_tree.
And call it here like local is_tree = $(_is_inside_git_tree)
So, the tasks are:

  • Move this function into utils file and git section
  • Test it, both scenarios: inside and outside

if [[ ! -d ".git" ]] || [[ ! -d ".git/hooks" ]]; then
set +e
git rev-parse --is-inside-work-tree >/dev/null 2>&1
if [ $? != 0 ]; then
Copy link
Owner

Choose a reason for hiding this comment

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

And here we can work with is_tree variable initialized above.

@sobolevn
Copy link
Owner

sobolevn commented Dec 9, 2016

Sorry for the failing build, I should fix the building process on OS X.

@timchurchard
Copy link
Contributor Author

Thanks for the review comments. I've made the changes you suggested. The _is_in_git_tree function now lives in the # VCS section in the _utils/_git_secret_tools.sh and the set +e has been removed.

@sobolevn sobolevn merged commit 61cd067 into sobolevn:develop Dec 9, 2016
@sobolevn
Copy link
Owner

sobolevn commented Dec 9, 2016

Done! Thanks again.

I am planning to make a new release just around the New Year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants