grumphp icon indicating copy to clipboard operation
grumphp copied to clipboard

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

Open drupol opened this issue 2 years ago • 1 comments

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:

  • [ ] Are the dependencies added to the composer.json suggestions?
  • [ ] Is the doc/tasks.md file updated?
  • [ ] Are the task parameters documented?
  • [ ] Is the task registered in the tasks.yml file?
  • [ ] Does the task contains phpunit tests?
  • [ ] Is the configuration having logical allowed types?
  • [ ] Does the task run in the correct context?
  • [ ] Is the run() method readable?
  • [ ] Is the run() method using the configuration correctly?
  • [ ] Are all CI services returning green?

drupol avatar Jul 08 '22 13:07 drupol

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?

veewee avatar Aug 02 '22 15:08 veewee

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.

drupol avatar Aug 22 '22 06:08 drupol

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.

veewee avatar Aug 22 '22 07:08 veewee

You're right, thanks for the enlightenment.

Have a good day !

drupol avatar Aug 22 '22 07:08 drupol