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

pre-push hook doesn't work #165

Closed
RomanPakhomov opened this issue Mar 11, 2021 · 8 comments · Fixed by #561
Closed

pre-push hook doesn't work #165

RomanPakhomov opened this issue Mar 11, 2021 · 8 comments · Fixed by #561
Labels
waiting for response We need more details or confirmation

Comments

@RomanPakhomov
Copy link

RomanPakhomov commented Mar 11, 2021

Hello everyone! I have little issue connected wtih lefthook. Some information:

  1. NodeJS envoirement
  2. Version "@arkweid/lefthook": "^0.7.2"
  3. My lefthook.yml contains next content pre-push: commands: test: run: npx cypress run
  4. Then I tryed to trigger lefthook manualy, I see next result (I change some files in src directory):
    image
  5. Command npx cypress run work correctly

I think that issue connected with git setup maybe, but I checked .git/hooks directory, and see correct pre-push file.

#!/bin/sh

if [ "$LEFTHOOK" = "0" ]; then
 exit 0
fi

if [ -t 1 ] ; then
 exec < /dev/tty ; # <- enables interactive shell
fi


cmd="lefthook run pre-push $@"

if lefthook -h >/dev/null 2>&1
then
 eval $cmd
elif bundle exec lefthook -h >/dev/null 2>&1
then
 bundle exec $cmd
elif npx lefthook -h >/dev/null 2>&1
then
 npx $cmd
elif yarn lefthook -h >/dev/null 2>&1
then
 yarn $cmd
else
 echo "Can't find lefthook in PATH"
fi

@DmitryTsepelev
Copy link
Contributor

Hi @RomanPakhomov! Did you check out the Troubleshooting guide? What is the output of lefthook -v run pre-commit?

@DmitryTsepelev DmitryTsepelev added the waiting for response We need more details or confirmation label Apr 26, 2021
@ShPakvel
Copy link

ShPakvel commented Oct 5, 2023

@DmitryTsepelev We have the same issue for pre-push hook. I checked out the Troubleshooting guide. skip_empty: false suggestion from it doesn't work as well. I created separate group code-quality, which is totally the same as pre-push, and it works smoothly without additional skip_empty: false option set per command. But it is more like workaround. I don't see the point having duplicated logic. Maybe alias for groups at least. Though more correct from my point of view is pre-push should have no limitations or side effects like this. I would expect option skip_empty: false to work, or some forcing flag on group run like lefthook run pre-push --force.

@mrexox
Copy link
Member

mrexox commented Oct 5, 2023

Hey @ShPakvel , now lefthook wants push files to exist or skips the check. Could you share your config and explain your use case with more details?

@ShPakvel
Copy link

ShPakvel commented Oct 5, 2023

Hey @mrexox! It can be simple like this:

pre-push:
  follow: true
  piped: true
  commands:
    1_pronto:
      run: bundle exec pronto run --exit-code -c origin/master
    2_rspec:
      run: bundle exec rspec
    3_rswag:
      run: bundle exec rails rswag
    4_open_api_validation:
      run: bundle exec rails openapi:validate
    5_bundler_audit:
      run: bundle exec bundle-audit check --update

And it skips even with option skip_empty: false like this:

pre-push:
 follow: true
 piped: true
 commands:
   1_pronto:
     skip_empty: false
     run: bundle exec pronto run --exit-code -c origin/master
   2_rspec:
     skip_empty: false
     run: bundle exec rspec
   3_rswag:
     skip_empty: false
     run: bundle exec rails rswag
   4_open_api_validation:
     skip_empty: false
     run: bundle exec rails openapi:validate
   5_bundler_audit:
     skip_empty: false
     run: bundle exec bundle-audit check --update

Idea is that we want to be able to run all these check with the same single command lefthook run pre-push instead of doing it manually one by one even during development process (when files is not in stage yet nor committed).
As I wrote, we created separate group code-quality that runs the same commands as pre-push. And we use it during development.
The topic I raise here is: why we have to do duplication and remember to adjust all checks in several groups? It is against DRY principle. And in terms of Single Responsibility, it is the same responsibility / feature of "code-quality checks". We just want to be flexible enough to run it not only as protection before push (in pre-push hook), but also reuse it during development at any moment.

UPD: Thoughts about more correct approach described in the next comment.

@ShPakvel
Copy link

ShPakvel commented Oct 5, 2023

@mrexox My personal view is that we don't want to use skip_empty: false as static configuration in commands for this pre-push case. Because it will apply always. And we still want to skip run on git push if there is no related files changed.
I think what we miss is the --force flag on run, like lefthook run pre-push --force. This will allow us dynamically decide if / when we want to run commands, e.g. during development process.

@mrexox
Copy link
Member

mrexox commented Oct 6, 2023

I see. This looks like a new feature. So, you want to prevent skips if there's --force flag passed. I will prepare this feature in a few days.

@mrexox
Copy link
Member

mrexox commented Oct 6, 2023

Please, check out v1.5.1 of lefthook

@ShPakvel
Copy link

ShPakvel commented Oct 6, 2023

@mrexox It works smoothly. 🎉 ❤️ Thank you for so quick response and actions! 💯 🙇🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for response We need more details or confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants