phpro / grumphp

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

No tasks runs when no files changed #33

Closed igormukhingmailcom closed 9 years ago

igormukhingmailcom commented 9 years ago

Hi.

When I run php ./vendor/bin/grumphp git:pre-commit manually (and have no changed files) - I see green man even shouldn't. So, no tasks executed.

But tasks such as 'phpunit', 'phpspec', 'behat' need to be executed.

veewee commented 9 years ago

You want GrumPHP to validate the full codebase? Currently we only run tasks when files have changed. I am thinking of some continious integration commands that can run all configured tasks on the codebase. More info at #20

igormukhingmailcom commented 9 years ago

Tasks like phunit/behat/phpspec not executed on separate files. Its executed as is and any file list not passed to it as argument. So i think that independent types of tasks need to run even if no file changed.

veewee commented 9 years ago

If no PHP file has changed during pre-commit, why should these tasks be ran? Normally the tests and code should be in the previous committed state and shouldn't contain any errors.

For example: a front-end designer wants to add a CSS file, and all of a sudden he gets an exceptions on the unit tests which he does not know anything about. This doesn't seem to make much sense to me.

What is your opinion @aderuwe ?

aderuwe commented 9 years ago

@igormukhingmailcom I spoke to @veewee about this for a couple minutes this morning. How about we introduce a grumphp:ci (or something similar) command that runs all the tasks on all of the files, while we make it so that "normal" GrumPHP usage is based off of the changed files only? (For independent tasks, as you call them, that would mean we trigger the entire suit, but only if there's changed files.) What do you think?

igormukhingmailcom commented 9 years ago

@aderuwe I thing that its a great idea. I propose to name this command something like grumphp run (similar as php-cs-fixer run or phpspec run) or grumphp check, because this command will be used not only with CI.

igormukhingmailcom commented 9 years ago

Also option --skip=task will be helpful I think...

For example:

WDYT?

veewee commented 9 years ago

sounds good! This will require some context in which the tasks needs to run. Also: some tasks will not be able to run, like the blacklist task, since this only checks files that are being committed. So the tasks need to know in which context they can run. During commit, only the changed files should be added. During run, all files should be added. (Note: wouldn't this be slow on big codebases?)

This feature will require some thinking about the architecture of the project and propably BC breaks.

aderuwe commented 9 years ago

@igormukhingmailcom +1 on both counts!

veewee commented 9 years ago

The run command is implemented. This issue can be closed. The ability to skip tests is not implemented but is added as a new issue in #50.