By dman on
I've warned my team that I'll be doing code reviews and checks against bad practices. I'm thinking of cooking a few git pre-commit hooks to beat them with.
Here's an example I've started building (just as a strawman)
Has anyone else tried leveraging git for code quality anywhere? I see a few examples for PHP code lint, but I want to use stronger Drupal-specific practices.
I checked out
https://drupal.org/project/githook : which drupalizes drupal git hook management, but that's not the point, I want some hook recipes to install on our repo.
Is this a BOFH idea?
#!/bin/bash
# Checks for core hacks.
# Include this in your project as .git/hooks/prepare-commit-msg
# All you have to do to allow this is to flag your commit message with the text
# "CORE HACK" in caps, and it will go through.
# If your message does not contain that string, it will be rejected.
# the --skip-verify option will also work if you are just local and lazy.
WARNING_MESSAGE="CORE HACK";
echo "Running pre-commit hook to check if you are being silly";
# $1 is the filename where the text of the message is currently held
if grep "$WARNING_MESSAGE" $1 ; then
made_excuse=1
fi
# Check the files being committed.
git diff --cached --name-status | while read st file; do
# Skip deleted files.
if [ "$st" == 'D' ]; then continue; fi
#echo "You are working on file: $file";
bad_boy=0;
# This list is short and rough just to spot the worst offenders.
# Things in root dir are your own call.
for pattern in "^includes" "^modules" "^misc" "^themes" ; do
if [[ $file =~ $pattern ]] ; then
echo "BAD BOY - Don't hack core."
echo "The file change you propose to $file is in the path $pattern . I won't allow that."
bad_boy=1;
fi
done
if [ $bad_boy ] ; then
if [ $made_excuse ] ; then
echo "Well, OK. Just this once then"
else
echo "If you can figure out how to turn off this warning, then I'll trust you know what you are doing."
echo "Until then, this commit will not go in. Hint: read about git hooks."
exit 1
fi
fi
done
Comments
Replying to myself,
Replying to myself, https://drupal.org/project/dcq does what I want, though I'm thinking of using it as a base for something more interactive and chatty
.dan. is the New Zealand Drupal Developer working on Government Web Standards
I suggest just doing
I suggest just doing code-review pre-commit, and merging changes yourself after approving them. Doing reviews post-commit never works out.
Gerrit or GitHub PR's both work fine for that.
There is only one of me, and
There is only one of me, and I'm not going to be able to insert myself between the crew and "getting thing done".
I want to just start with automated codesniffer and bad practice analysis scans that send them compliants. If that doesn't help, the complaints will be published to the rest of the team for review. Or something.
Honestly, the things I am looking at include
: raw SQL in tpl files
: hardcoded references to nids in css file
: undocumented contrib module hacks
: using views_query_alter to change the behavior of one display for mysterious reasons
as well as the normal code style things like missing indenting and spaces, missing @ file descriptions and undocumented functions
I want a blanket "Stop doing things like that" automated review before I can ever get on to the "this could be better" manual reviews!
.dan. is the New Zealand Drupal Developer working on Government Web Standards
_
For detecting contrib module hacks there's also https://drupal.org/project/hacked - it breaks the all-in-one model but it looks cool.
Yeah, 'hacked!' is something
Yeah, 'hacked!' is something to use at the beginning. I don't want to blame anyone for adding a *stable* contrib module that has some style issues. I'll need to only do code reviews on custom stuff.
I need to start some sort of project-wide health check:
... sort of thing.
So I get a feel for the minefield before I pick it up..
.dan. is the New Zealand Drupal Developer working on Government Web Standards
Drupal pre-commit
We made a similar pre-commit hook as well. Please see it here https://github.com/geraldvillorente/drupal-pre-commit/blob/master/pre-co...
We are still brewing it to add more capabilities.
Thanks
drupal | father | husband | bike | beer
Thanks. There are a good
Thanks. There are a good bunch of ideas there!
.dan. is the New Zealand Drupal Developer working on Government Web Standards