This is part one of a four-part post. The rest of the proposal can be found at the following links:
- Part 1: Today's Project Application Process
- Part 2: Where the Existing Process Breaks
- Part 3: The Proposal
- Part 4: Sanity Check - Wait a Minute Here ...
The 'Coles Notes' summary of the actual proposal can be viewed HERE.
Or, if you prefer, download a copy of the entire proposal in Word format.**
Part 4: Sanity Check - Wait a Minute Here ...
If We're Still Doing Manual Code Reviews, How Does This Change Anything?
If it seems that the process of going from ‘Pending’ to ‘Certified’ looks an awful lot like today’s ‘Full Project Application’ process … well, you’re right. If that’s the case, and this proposal doesn’t make drastic changes to the code review process itself, how can it possibly solve anything?
While most would agree that the current review process is broken, this does not infer that it is not necessary. Having a code review process helps to improve the general quality and stability of the Drupal contrib space, as well as ease the load on the security team by identifying major issues before they are exposed in the wild. And as work-intensive as it is … manual code reviews are really one of the best ways to identify these issues.
So how does the proposal in this document address the ‘new project application’ pain points, identified earlier?
Clears the Backlog (Reducing Timelines)
While the current process has a rather large backlog of applications, with average response wait times fluctuating between 4 and 7 weeks at the extreme, the truth is that the total number of projects in ‘needs review’ status at any given time has actually remained relatively stable (between 160 and 180) for the last 6 months. This statistic would seem to infer that the number of reviews being performed is roughly keeping pace with both new applications entering the process, and projects being moved from ‘needs work’ to ‘needs review’ by existing project applicants.
As virtually none of the existing project applications have anything resembling ‘release’ branches in their sandbox repositories, the vast majority would immediately classify as ‘Un-certified’ projects. With no ‘review’ requirements for ‘un-certified’ projects, the existing applications would be closed with a description of the new project classes, instructions for how to create an official ‘-dev’ release in their sandbox repository, and an explanation describing how to apply for future project promotions once they meet the requirements.
With this, the existing (and imposing) application backlog is essentially wiped clean … and from that point forward, reviewers can focus on addressing new ”Certified” project promotion requests as they come in.
Reducing the "Sandbox Stigma"
By granting the ability to publish –dev releases within sandbox repositories, we eliminate the git-related concerns described earlier (such as module testers desiring tarballs), as well as reduce the stigma associated with sandboxes as not being ‘real’ projects.
Reduced Number of Applications (Reducing Timelines)
First time contributors have a number of different motivations for applying for ‘full project’ status. For some, it is the desire to claim their project namespace. For others, the ability to support ‘release’ tarballs may be a motivating factor. Some may covet the unrestricted ability to create additional projects in the future, while others may simply want to share an existing piece of code with no intention of developing additional modules in the future. And for some contributors, it may simply be the sense of accomplishment or acceptance that comes along with having their project approved.
Whatever their motivations, each of these contributors must go through the complete “full project application process” in order to achieve their goals in today’s environment. However, not all are looking for the full suite of benefits that a project approval delivers. For example, a quick look through the existing contrib library suggests that many maintainers are completely happy with ‘perpetual beta’ projects.
By limiting the code review requirement to those maintainers with ‘release-ready’ projects, and simplifying the requirements for those maintainers simply looking to release a tarball or claim their project namespace, it is expected that the volume of ‘certified’ project applications will come down relative to the existing rate of ‘full project’ applications being experienced. This in turn helps alleviate the pressure put on the review team.
Improved Application Quality
As this proposal should allow for more end-user testing and validation of contrib projects before they reach the ‘certified’ application point, and coupled with the fact that any low-hanging fruit should be addressed during the ‘un-certified’ to ‘pending’ stage, it is expected that the overall code quality of projects which reach the ‘certified’ application queue should be greatly improved. Not only does this simplify the actual review process, it also helps ease a common source of frustration for project application reviewers.
Increased Number of Reviewers
This proposal attempts to address the issue of ‘too few reviewers’ from two perspectives.
Firstly, validation of the requirements at the ‘pending’ stage are intentionally trivial to verify, and can be performed by virtually any member of the Drupal community.
Secondly, allowing sandbox releases and enforcing ‘release candidate’ soak periods should result in an increased number of end-users leveraging the functionality of the project prior to the ‘certified’ application… and by extension, increasing the pool of people who i) are familiar with the project, and ii) have a vested interest in seeing the project be granted ‘certified’ status.
While this won’t result in a huge influx of general reviewers joining the code review team by itself, it could result in increase in community members performing one-time reviews … and, in the process, helping them realize that the process of performing a review really isn’t nearly as difficult as it one might assume!
While this may not seem like much, it becomes a much larger impact when combined with the reduced number of applications coming through the queue in the first place.
Module Duplication
Module duplication is certainly a concern within the greater community … but the code review team often gets stuck with the role of ‘bad cop’ when it comes to enforcement. As discussed earlier, the module review check should really take place at the initiation of the development process, not during a post-development project approval step. As such, this proposal pushes some responsibility for enforcement of the ‘module duplication’ issue back on the general Drupal community.
That said, as a sanity check to ensure that module duplication has at least been considered, the proposal does suggest that one of the requirements of the ‘pending’ stage would be that the maintainer provide at least some indication that they have done a related module search, and provided an explanation of how their module differs from other existing contrib modules.
Some may argue that this proposal results in a relaxed enforcement of the module duplication check, relative to the existing process. This may be true … and while both are serious issues, I would counter that reducing the frustration experienced by new project contributors (and thus helping ensure a consistent stream of new contributors and innovation into the future) outweighs the need to reduce duplication within the Drupal contribution space.
Project Approval vs. Developer Approval
The proposal contained in this document deals primarily with ‘project approval’, and does not necessarily address the ‘developer approval’ component of the existing process. This is intentional, and supports the earlier argument that the two should be separated.
This proposal reduces the dependency on any sort of ‘developer approval’ process in order to create ‘un-official’ or ‘pending’ projects. Instead, it focuses on the project itself, in an attempt to minimize any personal or emotional reactions associated with criticisms of the code itself.
From a developer approval perspective, one could associate the ‘full project approval’ permission with the ability to promote projects from ‘un-certified’ to ‘pending’, and the ‘git administrator’ permission (or some new ‘certify projects’) role with the ability to promote projects from ‘pending’ to ‘certified’ … though the granting of these roles should be separated from the project approval process itself. (In the spirit of true separation, the definition of how to grant these new permissions is out of scope for this proposal.)
Never back to Square One
Finally, the staged progression as outlined in this document ensures that a new applicant is never sent back to square one. At the worst case scenario, their project will be stalled at the current stage (whether that be un-certified or pending) … but never will their existing contribution to date be discarded. This proposal attempts to eliminate any sense of ‘rejection’, focusing instead on ‘this is what you need to do next’ … in the hope of fostering a spirit of continuous improvement in new project contributors.
This is part one of a four-part post. The rest of the proposal can be found at the following links:
- Part 1: Today's Project Application Process
- Part 2: Where the Existing Process Breaks
- Part 3: The Proposal
- Part 4: Sanity Check - Wait a Minute Here ...
Or, if you prefer, download a copy of the entire proposal in Word format.**
Comments
Automation?
"validation of the requirements at the ‘pending’ stage are intentionally trivial to verify - Actually some of them could be performed automatically, with effort ranging from trivial to not-trivial on the part of the drupal.org developers.
"[...] and can be performed by virtually any member of the Drupal community" -We can help the applicant do a better job of doing them personally, with prominent "just-in-time" cues which distill the critical points and link to d.o information architecture.
The raw information about the process does exist, but, the critical points could be presented as the applicant is actually stepping through the sandbox creation and project application, with cues as well as links to the relevant handbook pages.
Let's look at some of the items identified in Part 2:
Pain Point 2: Quality of Applications
* Incomplete Applications
** "project applications without sandbox links [or] empty sandbox repositories" - A script can handle this, and notify the applicant. Similarly, sandboxes can be scanned for things like license files, third-party libraries, absence (in application description) of indicator of Drupal version, proper use of t(), matches of any variable_set()'s and schema to deletes in hook_uninstall, improper DB access functions, matching eval() or drupal_eval() to an explicit permission or grant, granting of a permission already defined by another module, presence of HTML without theme function, presence of JS without jQuery, just to address many of the items in Tips for ensuring a smooth review. There are probably more which come up regularly in reviews.
** "no description of what the module is actually supposed to do" - a prominent field description and possibly also a confirmation prompt can cue the user to think about whether they've satisfied this.
* Code Quality
** "often, the code is perfectly fine PHP code, but doesn’t take proper advantage of any of Drupal’s APIs" - as above, prominent cues along the way can cue the developer that this is likely to be something which will come up in their code review.
** Automate early code review by letting a hosted instance of the Coder module run on a sandbox automatically, cueing the applicant immediately to existing problems and cueing them to use Coder themselves locally.
Pain Point 5: Module Duplication
* The cues/prompts on the above "description of what the module is actually supposed to do" item might communicate to the applicant that duplication is a potential showstopper to their application. The cue should inspire them to both carefully search both projects and sandboxes (directly linking to the right effective search tools) and also directly address the existing modules in their description. The cue could make it very plain that such info in the description is something which reviewers are very likely to need, in order to effectively move the process along.
* "there was already an existing duplicate project in the contrib space, even though the duplicate was created in the meantime while their module was stuck in the application queue" - I believe that "module search" should reveal both projects and sandboxes. People start developing modules because they can't find a matching project, but also because they can't even discover that another party is already working on the same problem - and has even provided at least some code toward a solution.
Pain Point 6: Applicant Patience and Perceptions
* "As time passes without feedback, applicants start to wonder if perhaps their application was missed, or is being ignored … especially as there are no real expectations communicated back to applicants as to how long the application process may take" - Automate the priority changes described in "Review process for Full Project Applications: What to Expect", sub section "Application Review Timelines", and alert the applicant to the priority change. A notification and an actual action every 2 weeks (or sooner, if a human being actually arrives on the scene and takes action) will go a long way toward soothing the patience of applicants.
* "applicants [...] see official modules in contrib duplicating the very same issues that they are being told are show-stoppers for their application" - Anybody who finds in an existing project a problem which is of a kind which would kill a project application should have a way to flag the project, and there should be accountability on the part of those project maintainers for such. I'd almost go so far as to argue for some kind of "demotion" status, pending the same kind of review that a new applicant would experience. This would go a long way toward helping applicants feel less underpriveleged.
Pain Point 7: "Project" Approval versus "Developer" Approval
* "these two items are interdependent. If you approve the module, you approve the reviewer. If you deny the module, you deny the reviewer. There is no separation of the ‘project approval’ process from the ‘developer approval’ process" - Just as above (immediately previous point). This suggestion makes sense even if the entire proposal presented here is implemented - there still should be accountability, even following approvals.