phpro / grumphp

A PHP code-quality tool
MIT License
4.15k stars 434 forks source link

Check if PHP is available before trying to run Grumphp (wip) #1024

Closed drupol closed 2 years ago

drupol commented 2 years ago

Hi,

Do you think such minor update in the hooks are justified in Grumphp?

Thanks

Q A
Branch master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Documented? no

New Task Checklist:

veewee commented 2 years ago

Why would you want to skip the hooks in that case? Doesn't it sound more reasonable to fail so that the root cause can be fixed by the owner of the machine instead?

I don't see a big benefit of adding this check at this moment. In what situation would you use it?

drupol commented 2 years ago

Right, I understand that sometimes this is not really feasible using command in bash, I don't see any better option right now.

I don't see a big benefit of adding this check at this moment. In what situation would you use it?

Context: I'm using Nix locally for PHP development. Issue: Sometimes, I need to make a change in a file without entering an environment containing PHP and all my dependencies (nix shell github:loophp/nix-shell#env-php82-nts --impure). When I do that, git refuses to commit stuff because the hook is failing, because PHP doesn't exist. This is precisely the reason why I wanted to make this PR.

veewee commented 2 years ago

I'dd suggest to either

  1. Commit with -n flag in those cases
  2. Create your own git hook templates

I don't think this is something we should add to this repo nevertheless.

drupol commented 2 years ago

You're right, thanks for the enlightenment.

Have a good day !