rycus86 / githooks

Githooks: per-repo and global Git hooks with version control
MIT License
381 stars 20 forks source link

Bugfix: Finding git directories. #76

Closed gabyx closed 4 years ago

gabyx commented 4 years ago

To review. Not yet perfect.

GIT_DIR=$(cd "$EXISTING" && GIT_DISCOVERY_ACROSS_FILESYSTEM=0 git rev-parse --git-dir)

needs revisit. Return relative path??

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 511


Changes Missing Coverage Covered Lines Changed/Added Lines %
install.sh 16 19 84.21%
uninstall.sh 13 16 81.25%
<!-- Total: 29 35 82.86% -->
Files with Coverage Reduction New Missed Lines %
uninstall.sh 8 79.11%
cli.sh 10 88.72%
install.sh 57 78.09%
<!-- Total: 75 -->
Totals Coverage Status
Change from base Build 506: -2.6%
Covered Lines: 1897
Relevant Lines: 2252

💛 - Coveralls
rycus86 commented 4 years ago

Do we have any other is_git_dir checks anywhere? Might be good to keep them consistent and doing the same thing.

rycus86 commented 4 years ago

Thanks a lot for looking into this btw!

gabyx commented 4 years ago

Please see the changes now.

Btw: Would it make sense to collaborate all copy&paste functions into a general.sh which we also inject during commiting. so we only need to fix bugs once, would you mind refactoring this? Would help in the future.

rycus86 commented 4 years ago

Changes look good, just waiting for Travis then I'll merge it.

I'll think about the shared snippet to insert, it's not only a good way to share code but also to share bloat that some scripts wouldn't have needed otherwise. 🙂

gabyx commented 4 years ago

Do we have any other is_git_dir checks anywhere? Might be good to keep them consistent and doing the same thing.

we only have is_git_repo tests and these are ok I think. Other git_dir checks I dont know

gabyx commented 4 years ago

I have not tested it locally on my machine :-) (on windows, unfortunately.) If it does not work, I check tonight...