rycus86 / githooks

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

Make cli.sh work directly from the release clone #104

Closed gabyx closed 4 years ago

gabyx commented 4 years ago

I am waiting for a response on this question: https://stackoverflow.com/questions/62218749/does-git-create-new-files-during-merge-or-overwrites-existing-ones

gabyx commented 4 years ago

Seems to work https://travis-ci.org/github/rycus86/githooks/builds/695079360 Good to go :+1:

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 672


Changes Missing Coverage Covered Lines Changed/Added Lines %
install.sh 26 34 76.47%
<!-- Total: 26 34 76.47% -->
Files with Coverage Reduction New Missed Lines %
uninstall.sh 1 83.84%
cli.sh 53 82.87%
<!-- Total: 54 -->
Totals Coverage Status
Change from base Build 670: -2.2%
Covered Lines: 2055
Relevant Lines: 2517

💛 - Coveralls
gabyx commented 4 years ago

Thanks for mergin

gabyx commented 4 years ago

@rycus86 : Wait... I am unsure if we really want to go down the road of having files executed in the release folder (which is updateable at the same time) because we rely on git that it really creates a new inode (which is the case)...

Therefore, this would make the whole discussion about placing wrappers instead of the whole scripts obsolete, as we dont want to rely on this behavior... However taking out single install repos is not a bad change I think, it just blows up the whole logic and is probably worth taking out...

@matthijskooijman Any toughts about this?

I mean we already rely on this behavior for the install.sh script sadly

gabyx commented 4 years ago

We could as a solution maybe before any update happens copy the install.sh to a temporary location and execxute from there exactly at this line: https://github.com/rycus86/githooks/blob/bf018cb078ae651667d26f28404ab80bb71e77a6/install.sh#L41

gabyx commented 4 years ago

For me its good to go :+1:

rycus86 commented 4 years ago

This is conflicted, otherwise looks good to go.

gabyx commented 4 years ago

Rebased and corrected the test094 by adding a proper install again after failing because of the single flag...

gabyx commented 4 years ago

All test pass > Good to go :+1:

rycus86 commented 4 years ago

Capped to long lines in echos to stdout, to make it more readable

I think this changes some of the indentation logic that was used elsewhere (line continuation to be indented if I remember correctly), but OK to look at that later maybe.

Looks good to me, happy to merge once the conflict is resolved.

rycus86 commented 4 years ago

Capped to long lines in echos to stdout, to make it more readable

I think this changes some of the indentation logic that was used elsewhere (line continuation to be indented if I remember correctly), but OK to look at that later maybe.

Looks good to me, happy to merge once the conflict is resolved.

gabyx commented 4 years ago

Ah is that the style? I just seen this only a couple of times ? all other warnings and such dont use special line contin. indentation Shit still a conflict , I rebased hmmm. Lets see...

Von meinem iPhone gesendet

Am 07.06.2020 um 00:33 schrieb Viktor Adam notifications@github.com:

Capped to long lines in echos to stdout, to make it more readable

I think this changes some of the indentation logic that was used elsewhere (line continuation to be indented if I remember correctly), but OK to look at that later maybe.

Looks good to me, happy to merge once the conflict is resolved.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

gabyx commented 4 years ago

Rebased to master