sjparkinson / static-review

:hand: An extendible framework for version control hooks.
MIT License
321 stars 27 forks source link

Take account of the git staging area #3

Closed jderusse closed 10 years ago

jderusse commented 10 years ago

I think you forget a tricky case: The reviewers will check the file in the current state on the fileSystem and not the file as it will be committed. It can occurred with a partial git add, but it will often append when you fix the violation and forget to add the file before the commit. sample:

vim index.php
# edit file and add a violation
git add index.php
git commit # pre-commit reject the commit
vim index.php
# edit file to fix the violation
# notice: the modification is not added with "git add"
git commit # pre-commit do not reject the commit (file on the hard drive is OK but committed file is not)

To avoid that, you should not run reviewers directly on the original file, but on a copy of the snapshot of the file as it will be commited. You can retreive that snaphost with git shot :path/to/the/file Here is a bash version of a solution (not tested):

$TMP_DIR = /tmp/$(uuidgen)
mkdir -p $TMP_DIR
for FILE in $SFILES
do
    mkdir -p $TMP_DIR/$(dirname $FILE)
    git show :$FILE > $TMP_DIR/$FILE
done
./vendor/bin/phpcs --standard=PSR1 --encoding=utf-8 -n -p $TMP_DIR

rm -rf $TMP_DIR
sjparkinson commented 10 years ago

:+1: This is something I've encountered myself actually!

Personally I have a habit of using git commit -am now, but totally see this an an issue worth resolving. Should be simple enough to implement (famous last words right?).

sjparkinson commented 10 years ago

This should now be address in the dev-master branch as of 46be5b0e860f957d4c4e75a4a455ab738bf9db71.