Add new comment
by Les Peabody
Cross-posted with permission from Genuine Interactive
Les is a web applications engineer at Genuine Interactive. He is a frequent Drupal community contributor. Genuine’s PHP team works on projects in a range of industries from CPG, B2B, financial services, and more.
As a contributor to the Drupal community, this post will detail Les’ method for enforcing Drupal coding standards by using the Coder and Coder Review modules: Drush and Git, which will make coding to Drupal standards a direct requirement during the software versioning process.
The Drupal community, as represented by a handful of statistics taken straight from Drupal.org:
- 27,098 Modules
- 2,012 Themes
- 827 Distributions
- 33,875 Developers
Also, 1,074,291 people in 230 countries speaking 181 languages are using Drupal. These numbers represent the documented basis of the Drupal community in its entirety.
I would like to take a moment to focus on one of these statistics: 27,098 contributes modules. That’s a lot of code being written by approximately 33,875 developers. Chances are that each of these developers have their own unique coding style. No artist paints with the same set of brushes, after all. However, it also means that a lot of those modules coding styles will look completely and totally bizarre to a lot of other Drupal contributors.
Coding standards are important, especially to a project as large as Drupal. Code that follows standards leave no surprises for a developer coming into a project for the first time. They can seamlessly delve straight into the codebase and get things done immediately, without the need for interpreting strange variable naming conventions or wonky code formatting.
Drupal implements a variety of standards. The rules of each standard vary from simple formatting rules to rules that check your code for security implications. Every Drupal module should adhere to these standards. If all modules did this, then Drupal itself would be a more secure, performant, and readable codebase.
The IDEs and editors we write our code in can handle real-time code checking for these standards rules, and we should use these tools during development, but they are merely passive tools and not the focus of this post. You’re still free to save your code however you like, commit that code, and then push that code back up to whatever repository you have it stored in. If you are truly committed to Drupal, then you should guarantee the code you contribute back be standards compliant.
How does one guarantee this? Simply make it impossible to commit or push your code until the code you’ve worked on is standards compliant.
Using the Coder, Coder Review, Drush, pre-commit and pre-push hooks, we can ensure that all code destined to a central repository somewhere is 100% Drupal standards compliant. This post will focus on Drush integration with Git, and will not go into detail on using the Coder module through Drupal’s admin UI.
Coder, Coder Review, and Coder Sniffer Modules
The Coder module is essentially a plugin manager. Coder Review and Coder Sniffer (which come with the Coder module), are the standards packages which contain the actual standards tests. Following are the review tests we are going to care about for now:
- Drupal CodeSniffer
- Drupal Security Checks
- Drupal SQL Standards
- Drupal Commenting Standards
- Drupal Coding Standards
There’s no real need to go into the specifics of what these test for, but they’re pretty apparent by just reading their names. Perhaps another post in the future will be allocated for explaining each of these standards reviews in more detail.
Coder Review Integration with Drush
I will assume you have experience with Drush, and so I won’t go into detail about what it’s used for. I’ll simply say that Drush is the best thing to happen to Drupal administration. You can do many awesome things with it… like running code reviews from the command line. The following line is an example of such:
drush @local.drupal coder janrain –minor –ignore –no-empty –reviews=sniffer,security,sql,comment,i18n,style –ignorename –ignores-pass
That line tells drush to invoke the coder module and run each of the reviews listed against the janrain module, enable ignore handling (–ignore), do not display reviews that pass (–no-empty), show the ignore handler name if used (–ignorename), and test all levels of rules (–minor). The –ignore-pass option tells Coder to not count ignore warnings towards the shell exit code status (more on this when we cover pre-commit and pre-push hooks). If you are knowingly ignoring code, then the ignore warning generated should not count towards the failure status of the command. There is a patch I wrote sitting at https://www.drupal.org/node/1974654 that deals with this situation.
Also, any files you list at the end of this line will also have the reviews run against them. For a full list of options available to the Drush coder command, simply run Drush help coder.
Integrating with Git
Okay cool, so we can run code reviews from the command line. Coder thought ahead and decided that when executing reviews via Drush, a shell exit code will be returned. This exit code is equivalent to the number of all the warnings and errors produced during the review process. As you may or may not know, when you execute a shell command, the command is considered to be a failure if a code other than 0 is returned from the command. Therefore, if a code review produces any errors or warnings, the command will fail in the shell.
Consider this when you think about wrapping this command into either a pre-commit or pre-push hook. Essentially, if the review fails, it is impossible to get your code to be either committed locally or pushed remotely, therefore making it impossible to contribute your code back unless it’s 100% compliant with the reviews specified through the drush command, and against the appropriate review level (–minor, –major).
For the sake of example, let’s focus on the pre-commit hook. This is a pretty strict implementation since your code will need to be compliant with the reviews you’ve specified for each commit, so if you commit code very frequently, you may want to utilize a less intrusive method and rely on a pre-push hook instead.
Following is the code from two files – pre-commit and pre-commit_janrain. This code is produced (more or less as is, with some modifications based on local environment) by running the review command along with the –git option.
Bringing it all together…
With the previous two files present in your local Git repositories hooks directory, your versioning process is now enabled for enforcing coding standards prior to your code making its way to a central repository somewhere. You’ll need to modify both files based on your local environment such as adjusting the top level variables in each file. You may also want to customize the Drush command that is being executed as well, since this may also depend on your local environment if you’re using things like site aliases.
The file pre-commit is just a generic handler used to load all of the other pre-commit hooks you’ve created. It relies on naming your pre-commit hooks with the file naming convention of pre-commit_hookname.
The file pre-commit_janrain is where the magic happens. Simply stated, this hook will run reviews against the janrain module every time a commit is made locally, as well as running the same reviews against every file that has been modified between the last commit and the current HEAD. This is exactly what we want. Any file you’ve touched, assuming it isn’t ignored, will need to be 100% Drupal standards compliant in order for the commit to be successful. If any warning or error is identified in the janrain module or the files that were modified, then the commit will fail and you’ll need to correct the errors.
Really, it’s just that simple. A task so simple, yet has major impacts on the Drupal community as a whole. If we all code to standards, Drupal will be a stronger, more performant, more secure platform. Drupal will continue to grow and strengthen it’s community due to the quality of it’s codebase. These are things every Drupal developer should care about.
This method of standards enforcement was utilized with the need to produce a solid codebase for a new module we wrote for a project we are involved in with Acquia.