Last updated March 7, 2017.

When submitting your application, there are a number of things you can do to help ensure a smooth review. The list below contains a number of suggestions which, while not guaranteed to result in a successful application, should at least make your application easier to review (and thus help avoid some of the common discussions/pitfalls which can delay the review process).

Quick Tips

Application checklist
A checklist which basically covers all points of the application process a reviewer will take care of. Resolving these common issues can be a true time saver in the application queue.
Review bonus
Make sure that you have reviewed three other project applications and linked your review comments in the issue summary of your application. See the review bonus program for help on that.
Include the intended Drupal version (D6, D7 or D8) within your project application post.
Some reviewers may prefer to review applications for one or the other, given their own level of experience with the D6, D7 or D8 APIs.
Avoid including any (non-GPL) 3rd party code or images.
If your project requires the use of third party libraries, provide installation instructions directing the user to where they may find the library, and how to install it.
If you are including 3rd party libraries or images in your project, you will need to be granted an exception by the Drupal Licensing Working Group (LWG). Exception requests should be posted in the LWG's issue queue with the component "Exception Request". You may apply for an exception while your project is still a sandbox. If you are including 3rd party materials in your project, just must provide a link in your application post to the issue in the LWG's issue queue where the exception grant is on public display.
Search for similar modules, and explain how your application is different.
This demonstrates that you have taken the time to look for existing solutions, and an awareness of Drupal's 'collaboration over competition' ethos.
If you do find existing modules which are similar or related to the functionality of your project (even if only in name), expect reviewers to suggest instead providing your module as a feature or patch to the existing project. Be sure you've read the information at this link, and include your reasoning for the 'new module' approach in your application.
Pre-test your script for xss vulnerabilities.
If you're not using check_plain(), check_markup(), theme_placeholder(), or filter_xss(), chances are that your code is vulnerable!
Try entering <script> alert('xss!'); </script> into every text field used in your module, and browse around looking for popups.
Use t() on every string which is a part of your user interface.
Exceptions to this are in Menu descriptions and titles, Schema descriptions, and when invoking watchdog().
If your module defines any variables, remove them in hook_uninstall().
For D6 modules, hook_uninstall should also be used to remove any database tables created by the module. (This occurs automatically in D7 and D8.)
Include some information about your background and experience with Drupal in your application.
Providing background information gives reviewers some extra context which they can use while reviewing your application. This also allows them to tailor feedback to the experience level of the applicant, providing additional links and background to applicants who are less familiar with the 'Drupal' way of doing things.
Chip in while you wait!
Unfortunately, the application queue does occasionally experience a large backlog, and applications may sit in the queue for a few weeks at a time before getting reviewed. While you wait, consider joining the Code Review Group and contributing a few reviews of your own. Providing a solid review of another's work can help demonstrate your understanding of the Code Review principles, and some reviewers may be willing to help fast-track your own application when they see you chipping in.

Common Issues

Module submission reviews have pulled up a number of similar problems. The list has grown to almost become a project application FAQ, containing the most common comments made after a review. Addressing each of these in advance will almost certainly help your chances of a successful application.

Your module contains non-secure code.
In particular:
Your code is outputting user-submitted content without to first pass it through check_plain(), or theme('placeholder'), or check_markup(), or filter_xss().
No piece of user-submitted content should ever be placed as-is into HTML.
Your code doesn't use the proper database access functions, or concatenate data directly into SQL queries.
The access to the database should be done through the database access functions.
Code like db_query('SELECT * FROM {table} t WHERE t.name = ' . $_GET['user'])
is an open gate to security issues, as the content of $_GET['user'] could be ' OR 1, and would cause the query to show all the rows contained in the table.
Your code has a lot of string literals but not a hint of t().
Every string used in the user interface needs to be translated through t(). The system adopted to translate strings uses a script that collects the strings that are passed to t(), and puts them in a template file that will be used (directly, or through the translation server, such as the one running in http://localize.drupal.org) to create the translation files; that is why the first argument of t() must be a literal string containing English words, but not containing quote escapes.
In some cases, the function t() doesn't need to be called; these cases include:
  • Menu callback descriptions, and titles; Drupal core code takes care of passing those strings to t().
  • Schema descriptions.
  • When invoking watchdog(). The first two arguments are the same arguments you would pass to t(); watchdog() will call t() with the first two arguments.
Your code allows a user to insert PHP code, but it doesn't provide a specific permission users must have in order to use PHP code.
Using eval() or drupal_eval() in your module's code could have a security risk if the PHP input provided to the function contains malicious code. Even in the case a user needs to have an administrative permission to use PHP code, it is better to have a more specific permission to use PHP code.
Drupal 7 uses a more generic permission (use PHP for settings) that should be used from any modules that allow a user to use PHP code in their settings pages.

For an example of currently used code, see what block.module does in block_perm(), and block_admin_configure() (for Drupal 7 code, see block_permission(), and block_admin_configure()).

Your module shows no hint of Coding Standards.
Although sticking hard and fast to the standards is not an absolute requirement, plain sloppy and poorly presented code is not accepted. Try to follow the standard if at all possible and try to make the code clean and readable. If a reviewer is having a hard time reading your code it is likely that a number of other people will too. Remember, by placing your code on drupal.org you are letting the World see your code. Impress people not only with it's awesome functionality but the quality of coding too!
To easily find where the code doesn't follow the Drupal coding standards, you can use an automated tool:
  • If you want to review code that you have committed through Git to your sandbox then you may use pareview.sh/. It is the product of the PAReview.sh project.
  • As an alternative, you can manually install and run Coder (and Coder Sniffer), which is what PAReview.sh uses.
Your module doesn't remove the Drupal variables it defines.
Drupal 6 modules can implement hook_uninstall() to remove Drupal variables it defines, and the database tables it installed; in Drupal 7 and 8, database tables defined from a module are automatically removed.
Your module should cooperate with other modules.
In particular
  • Two modules should not define the same permission. When a permission is already defined from a module, the other module doesn't need to define it; it should just use it. That is true whenever the permission is defined from a Drupal core module, or a third-party module.
  • When a module alters a database table defined from another module, it should also implement hook_schema_alter(), in order to allow the other module to correctly work.
  • If a module depends from a database table defined from another module, then it should declare its dependency from the other module rather than creating the database table when the module that creates that database table is not enabled.
  • Each module should respect its own namespace, and correctly name the functions, global variables, Drupal variables, and constants it defines.
  • When removing the Drupal variables it defines, it should not use code similar to the following:
    /**
     * Implements hook_uninstall().
     */
    function mymodule_uninstall() {
      drupal_uninstall_schema('mymodule');
      db_query("DELETE FROM {variable} WHERE name LIKE 'mymodule\_%'");
    }
    

    Such code would remove also the Drupal variables defined from the module mymodule_extention.module, for example.

Your module outputs a lot of HTML but there's not a theme function in to be found in the code anywhere.
Those people around the world who theme really won't be your friend if they are forced to use your module.
Your module pulls in a lot of JavaScript code in-line without a hint of jQuery involvement.
Try to use jQuery to play nicely with all the other jQuery code running on end users sites.
Your code does not include a license file and does not have comments indicating any other license than GPLv2+.
All projects hosted on Drupal.org are automatically licensed under GPLv2+, just as Drupal is itself. A copy of the GPL text (LICENSE.txt) will be added automatically for packaged downloads later.
Your code comes with files available from third-party sites, or that are part of another project.
Such files are generally not accepted. Files that are not licensed under GPL License are not accepted, even when the license is compatible with GPL License; also in the case the files are effectively licensed under the GPL License, they are still not accepted, if not in particular cases.
For more information, see Why drupal.org doesn't host GPL-"compatible" code, and 3rd party libraries on Drupal.org.
Incomplete Submission
Many people write good motivation messages and then mess it up right at the end with here's a link to my contribution, it's about 75% done. Please, when applying, supply code that you believe is complete. Attempting to review uncompleted work is difficult and time consuming. Reviewing completed work is much simpler as there is already a level of expectation in the reviewer. Try to make our job of reviewing your work a joy.

Motivations

Here is a list of common motivation messages that, more often than not, get declined/rejected. If your motivation message resembles one of these then please, think before applying.

Drupal is cool.
This motivation, together with others like I want to give back to the community, doesn't provide enough detail.
Instead, you are expected to provide:

A message providing information about yourself and your planned contributions.

  • Describe what modules, themes, or translations you want to maintain and why.
    Please provide as much information as possible along with appropriate links if available.
  • If your contributions implement existing functionality explain why you want to duplicate it or why you cannot extend any of the existing projects.
  • If you have been asked to co-maintain an existing module, please link to an issue where the current maintainer explicitly states so.
  • More information, the better.
I would like to fix bugs I find as I go or add new features to existing modules.
The correct way to leave feedback for a module is to use the patch issue/queue system. And, in fact, for Drupal Core, this is the only way.
I am planning to create modules / themes.
Someone who applies for access should have already a module or theme they consider ready for release. The purpose of the application is both to review code and verify the degree of understanding about how a Drupal module is written prior to review. Without a module or theme, there is no way to to perform a review.
Module 'Foo' no longer appears maintained so I have written a new version I would like to contribute.
Realistically this forms duplication and Drupal.org tries to avoid this where possible (see the previous link for why this is so). The best way forward in this situation is to provide feedback to the module with patches rather than strike out afresh.
If the module appears abandoned and patches sit by without review of commit, use the procedure for dealing with abandoned projects to ask to take over an orphaned module. We will still review your contribution if you take this route.
Module 'Foo' is just too complex so I have written a much simpler version.
This is just straight duplication of functionality and applications like this will almost certainly be declined. It is worth remembering that many modules start out simple also and, over time, grow into more complex systems. If you have the know how to write a "simple version" from scratch rather than work out how a complex module works then maybe your effort would be better spent making the existing complex module easier to use (improve it's UI and UX, write documentation, etc).

Comments

bailey86’s picture

It's a big help to be able to review your code properly yourself before committing it because the first reviews are usually pointing out coding standards issues.

There are three main tools which should all be used to check the code.

1. The coder and coder_tough_love modules.

These are the easiest to use - install the modules and run them from the modules list - remember to select 'coder_tough_love' and 'minor' to pick up as many issues as possible.

2. Use the pareview_sh module.

This is a bit more tricky to use. Basically, install the module then test your module with:

sites/all/modules/pareview_sh> ./pareview.sh ../module_being_tested

3. Use phpcs.

This is from:

http://drupal.org/project/drupalcs

which is a D7 only module - or you can install the pear library from

http://pear.php.net/package/PHP_CodeSniffer

Follow the instructions at:

http://drupal.org/project/drupalcs

to get it working.

This can then be run directly on the command line on the module code - it does not need to have a full Drupal instance installed.

phpcs --standard=DrupalCodingStandard --extensions=php,module,inc,install,test,profile,theme ~/tmp/module_being_tested/

bailey86’s picture

My previous post is not easy to use - and I've been finding conflicts.

I suggest using http://ventral.org/pareview as it now has code sniffer implemented as well as pareview which I assume is running coder as well.

So, all three tools in one place. I've put forward a request to make this the standard tool to use. http://groups.drupal.org/node/195303

bailey86’s picture

Please note there is a bug in coder_tough_love http://drupal.org/node/1364816 where is is wrongly failing function comment blocks.

Then, if you change your code to get it past coder_tough_love then it will fail against code sniffer or the online test at http://ventral.org/pareview

Probably best just to use http://ventral.org/pareview currently.

bailey86’s picture

After a discussion on the reviewers group it looks like using http://ventral.org/pareview would be a good way to test your code for coding standards errors.