pre-commit hook - use new and modified files about captainhook

[di-repo]

At the moment I am testing with the following hook setup:

 "action": "vendor/bin/phpcs $(git diff --cached --name-only --diff-filter=ACMR -z | xargs -0)",

I totally agree with @heiglandreas that it's better to verify before pushing, but the script will be far more complex.

[sebastianfeldmann]

This could get quite big, it should not, but it could.
So providing this placeholder upfront maybe is not the best idea.

To solve this we could improve our placeholder system.
We would have to allow placeholder that can be computed only if they where used in an action configuration.

If you don't want to wait, you can have a look at the linting action and create your own code sniffer action accordingly.

But I will look into the placeholder thing

[heiglandreas]

The other thing is that you will still get notices about parts of your chamged files that you never touched but that where broken beforehand. IMO it would be best to always have a clean codesniffer run before you start. And then only your changes will cause codesniffer to complain. If you are worried about the speed: I wouldn't run CS on every commit but only on push to be able to commit fast and also probably unclean code and then have a cleanup before I push. Or think about running such tests (perhaps with an autocleaner) in CI.

Git Hooks are no replacement for CI!

[heiglandreas]

And you probably want to have a look at https://github.com/exussum12/coverageChecker

[fuess]

Thanks for all responses. I agree we should do a codesniffer run on whole codebase, but I thought it would be nice to 'grow' into this, without a big task beforehand. Thanks @dimitar-ivanov-tryzens for the suggestion. Will try it out.

[heiglandreas]

That's what coverageChecker does. 😉

[fuess]

Ah, missed that one, thanks!

[sebastianfeldmann]

In version 5 this is available for pre-commit hooks.

"pre-commit": {
    "enabled": true,
    "actions": [
      {
        "action": "tools/phpcs.phar --standard=psr12 {$STAGED_FILES|of-type:php}"
      }
    ]
  }
}

{$STAGED_FILES} will get replaced by the list of changed or added files. You can filter the list by using the {$STAGED_FILES|of-type:php} modifier or change the glue between files with {$STAGED_FILES|separated-by:, } and of course you can combine both modifiers.

[torhoehn]

@sebastianfeldmann If there is no staged PHP file e.g. the phpcs command will result in

ERROR: You must supply at least one file or directory to process.
Run "phpcs --help" for usage information

Is usage of https://github.com/exussum12/coverageChecker the only way to handle this? Or would it be possible to skip the wohl action?

[sebastianfeldmann]

You can configure a Condition like this

{
  "action": "...",
  "options": {},
  "conditions": [
    {
      "exec": "\\CaptainHook\\App\\Hook\\Condition\\FileStaged\\OfType",
      "args": ["php"]
    }
  ]
}

With this Condition the Action will only get executed if PHP files are included in this commit.
For a more detailed explanation have a look at the documentation