git-pre-commit-hook-windows-filenames icon indicating copy to clipboard operation
git-pre-commit-hook-windows-filenames copied to clipboard

refactored

Open st-gb opened this issue 4 years ago • 8 comments

Hi Thomas.

Endlich habe ich Zeit und Muße gehabt, das umzusetzen (Deine E-Mail vom 04.Juli soll nicht umsinst gewesen sein ;-) ). Gucke Dir mal die Änderungen an und übernimm sie dann.

Gru

st-gb avatar Oct 20 '19 23:10 st-gb

@st-gb Sorry for the late response.

I like the per check error messages but I don't think that creating a separate file for the test function itself is good.

I'm replying with some more comments inline.

Please also:

  • Explain why you did the change in the commit message
  • Don't commit commented out code

t-b avatar Feb 17 '20 19:02 t-b

@st-gb Please write in english so that others can follow as well.

t-b avatar Feb 17 '20 19:02 t-b

Hi Thomas. So I have to update my branch with a new commit including your requested changes, right?

st-gb avatar Feb 19 '20 17:02 st-gb

@st-gb Yes please add a new commit or even better squash your new changes into the existing commit and force push.

t-b avatar Mar 23 '20 15:03 t-b

I wrote a concatenate function for the causes. But I would like the testing be callable also from outside the pre-commit hook.

st-gb avatar Mar 23 '20 17:03 st-gb

You can see the latest changes here: https://github.com/st-gb/git-pre-commit-hook-windows-filenames/tree/suggestion_by_StGb

Now we have to decide whether single shell file for pre-commit file or not. pro & contra: pro: test without changing the repository. contra: current path problems (when called from another directory than where the pre-commit resides maybe testIfWinValidFileName.sh can't be found)

st-gb avatar Mar 23 '20 17:03 st-gb

@st-gb Sorry for not getting back earlier. As mentioned I'd like to stick with single file as it is more convenient.

t-b avatar Jun 15 '20 20:06 t-b

I strongly prefer my solution with a dedicated shell script for both testing and changing because of "separation of concerns" principle. If you stick to your suggestion then I'd like to put code from my "testIfWinValidFileName.sh" into function(s) within the "pre-commit" shell script. Please do this in your repository at first. I need some more time to do this for my one because I am busy ATM.

st-gb avatar Jun 23 '20 16:06 st-gb

No reply for two years closing.

t-b avatar Jan 12 '23 19:01 t-b