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

ci: Added pre-commit-hook #1320

Merged
merged 11 commits into from
Nov 17, 2020
Merged

ci: Added pre-commit-hook #1320

merged 11 commits into from
Nov 17, 2020

Conversation

Sloox
Copy link
Contributor

@Sloox Sloox commented Nov 16, 2020

Fixes #1305

Needs to be tested locally to actually see a change.
It requires a once off execution of the link Hook sh file.
Simply try git commit and it should run detekt.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 16, 2020

Timestamp: 2020-11-17 16:12:54
Buildscan url for ubuntu-workflow run 368470824
https://gradle.com/s/vfidhavlwb6wk

@codecov-io
Copy link

codecov-io commented Nov 16, 2020

Codecov Report

Merging #1320 (072cf41) into master (f02fb4e) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1320      +/-   ##
============================================
- Coverage     79.64%   79.61%   -0.03%     
+ Complexity      736      735       -1     
============================================
  Files           237      237              
  Lines          4573     4573              
  Branches        792      792              
============================================
- Hits           3642     3641       -1     
- Misses          525      526       +1     
  Partials        406      406              

Copy link
Contributor

@jan-goral jan-goral left a comment

Choose a reason for hiding this comment

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

The linkHooks.sh was not executable so I have changed it using chmod +x.
After that, I have added empty lines to a random kotlin file and tested pre-commit

MacBook-Pro-jan-gogo:flank-review jan-gogo$ git commit -m 'Test pre-commit'
hint: The '.githooks//pre-commit' hook was ignored because it's not set as executable.
hint: You can disable this warning with `git config advice.ignoredHook false`.
[#1305-git-hooks 19bd8833] Test pre-commit
 1 file changed, 9 insertions(+)

So I have changed pre-commit to executable and tried test again.
This is what I get:

MacBook-Pro-jan-gogo:flank-review jan-gogo$ git commit -m 'Test pre-commit'
fatal: cannot run .githooks//pre-commit: No such file or directory

Strange

@piotradamczyk5 piotradamczyk5 self-requested a review November 17, 2020 09:35
@Sloox
Copy link
Contributor Author

Sloox commented Nov 17, 2020

The linkHooks.sh was not executable so I have changed it using chmod +x.
After that, I have added empty lines to a random kotlin file and tested pre-commit

MacBook-Pro-jan-gogo:flank-review jan-gogo$ git commit -m 'Test pre-commit'
hint: The '.githooks//pre-commit' hook was ignored because it's not set as executable.
hint: You can disable this warning with `git config advice.ignoredHook false`.
[#1305-git-hooks 19bd8833] Test pre-commit
 1 file changed, 9 insertions(+)

So I have changed pre-commit to executable and tried test again.
This is what I get:

MacBook-Pro-jan-gogo:flank-review jan-gogo$ git commit -m 'Test pre-commit'
fatal: cannot run .githooks//pre-commit: No such file or directory

Strange

Please retest it was a faulty shebang line.

@Sloox Sloox requested a review from jan-goral November 17, 2020 11:16
@Sloox Sloox marked this pull request as ready for review November 17, 2020 11:31
@adamfilipow92
Copy link
Contributor

Maybe we could add information about run linkHooks.sh before first commit to documentation in Environment setup section? It could help when for all new contributors. WDYT?

@piotradamczyk5
Copy link
Contributor

One small issue on Mac

.githooks/linkHooks.sh: line 2: @echo: command not found
Changing git config for hooks to .githooks
Complete!

I fixed it

@piotradamczyk5
Copy link
Contributor

set echo off?

fixed

Copy link
Contributor

@jan-goral jan-goral left a comment

Choose a reason for hiding this comment

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

I am not sure if pre-commit script will work on windows so linkHooks.bat probably is not necessary, but also I am not sure if any dev will use windows as the main os for development, so it's not a problem.

@mergify mergify bot merged commit 733e444 into master Nov 17, 2020
@mergify mergify bot deleted the #1305-git-hooks branch November 17, 2020 18:03
@Sloox
Copy link
Contributor Author

Sloox commented Nov 18, 2020

I am not sure if pre-commit script will work on windows so linkHooks.bat probably is not necessary, but also I am not sure if any dev will use windows as the main os for development, so it's not a problem.

I use windows and it works correctly 👍

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.

Git hook lint before commit
5 participants