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

Skip option does not work for pre-push hooks #817

Closed
PidgeyBE opened this issue Sep 17, 2024 · 1 comment · Fixed by #850
Closed

Skip option does not work for pre-push hooks #817

PidgeyBE opened this issue Sep 17, 2024 · 1 comment · Fixed by #850
Labels
bug Something isn't working

Comments

@PidgeyBE
Copy link

PidgeyBE commented Sep 17, 2024

🔧 Summary

The option to skip some hooks does not work correctly for pre-push hooks.
The check to determine if a branch is in merge state only works for pre-commit hooks.
One of the ways to check the state correctly for pre-push hooks is e.g. $(git log -1 --pretty=%P | wc -w) -gt 1.

Lefthook version

v1.7.15

Steps to reproduce

  1. Have a pre-push hook that should be skipped for merge and rebase
pre-push:
  parallel: true
  commands:
    lint-yaml:
      glob: '*.{yml,yaml}'
      run: yamllint -d relaxed {push_files}
      skip:
        - merge
        - rebase
  1. Check out a certain branch branch-x
  2. Merge master, git merge master
  3. Run pre-push hooks

Expected results

Because of skip: merge, the hook should be skipped

Actual results

The hook is not skipped.

Possible Solution

Instead of checking the presence of .git/MERGE_HEAD, in case of a pre-push hook, lefthook should check if the last commit was a merge commit or not. E.g. via $(git log -1 --pretty=%P | wc -w) -gt 1 in bash

@PidgeyBE PidgeyBE added the bug Something isn't working label Sep 17, 2024
@mrexox
Copy link
Member

mrexox commented Oct 18, 2024

I will add a separate skip option merge-commit so you can use it instead of merge. I think this is more useful than supporting different behavior depending on the git hook name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants