Open claudiu-cristea opened 1 month ago
Thanks for the PR.
I'm a bit confused: There is already a draft, so wouldn't it be better to start from 1 PR and try to get that as good as possible? https://github.com/phpro/grumphp/pull/1136
This is in a very early stage. Not ready for a review. I didn't want to mess with the previous PR
I wasn't aware of the other PR ☹️
No problem. Feel free to give your feedback on the initial PR so that we can try to get that one as good as possible.
Hm, I'll have to coordinate with @saidatom. The way we're getting the pushed files list is different
Also, in this PR there's a feature that allows to configure which Git hooks to be initialized. For me, personally, checking on each commit is too agresive. I only want pre push checks. But that's me, others might want on pre commit. Making it customizable makes sense
True, I think that part could be covered by making it configurable per task basis instead of on a global basis as commented here https://github.com/phpro/grumphp/pull/1136/files#r1608281196.
Not sure how easy it would be to make the 'default' configurable as well - but for BC sense it should at least be pre-commit
@veewee, I have discussed with @saidatom. Until we decide the way to go, I'll continue to improve this. Regarding https://github.com/phpro/grumphp/pull/1136/files#r1608281196, I need to understand more what you are proposing there. You mean, for instance, that phpcs
could run on a push while phpstan
on commit?
Regarding https://github.com/phpro/grumphp/pull/1136/files#r1608281196, I need to understand more what you are proposing there. You mean, for instance, that
phpcs
could run on a push whilephpstan
on commit?
Indeed: the hook script is activated on both commit and push. Which task is being executed will depend on a per task configuration with default if not set.
@veewee
Indeed: the hook script is activated on both commit and push. Which task is being executed will depend on a per task configuration with default if not set.
I think this is fixed now.
I'm trying now to add some tests for the new feature. It would help a lot to enable GitHub Actions for this PR
New Task Checklist:
run()
method readable?run()
method using the configuration correctly?