Evolution of the Project Application Process (Part 2) - Where the Existing Process Breaks

You are viewing a wiki page. You are welcome to join the group and then edit it. Be bold!

This is part one of a four-part post. The rest of the proposal can be found at the following links:

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.


Where the Existing Process Breaks

There are a number of pain points within the existing process. This next section will attempt to identify and elaborate on some of the larger issues that I have witnessed in my relatively short experience with the new project application process.

Pain Point 1: Timelines

Application to Initial Review

Ideally, a new applicant should not have to wait more than a few days before their application is reviewed. In practice, some applications are sitting idle in the queue for 4 weeks or more before receiving their first comments. This leads to scenarios where the application frustration isn’t ‘when will I be approved’ … it’s ‘when will I be reviewed!’.

Initial Review to Re-review / Orphaned Reviews

In an ideal world, a project review would identify any issues that need to be addressed, the maintainer would address those issues, and then the application would be approved.

However, in practice, this rarely occurs. Reviewers don’t necessarily catch all potential issues in the first pass, and the applicant could potentially take weeks to respond to issues pointed out in the review. Even when the applicant responds immediately, the ‘fix’ may introduce additional issues, the reviewer may miss the response, or the reviewer may simply get busy with some other unexpected initiative. As a result, applicants too frequently find themselves waiting weeks for a follow-up review after addressing any issues raised in an initial pass.

Some reviews may bounce back and forth between ‘needs review’ and ‘needs work’ a half dozen times before being approved … and at up to 4 weeks per cycle, this leads to significant delays in the approval process.

RTBC To Approved

Once a review has been marked RTBC, the process falls to a Git Admin to assess the quality of the review, ensure no major issues have been overlooked, and grant the applicant the ‘promote full projects’ permission. Given the level of knowledge and trust required within this role, it has traditionally been reserved for a limited number of trusted individuals … who, by that trusted nature, tend to be very busy with other initiatives.

After finally working their way through and finally being given the green light for approval, applicants sometimes find their applications sitting in RTBC status for a week or more … which is seen as just one more unnecessary delay in an already frustrating process … while they wait for one of the busy Git admins to finally happen along in a spare minute. In many cases, the Git Admin then catches something that was missed in the initial reviews, bumps the status back to ‘needs work’, and the cycle starts over again.

Implementing Improvements (i.e. General Inertia)

Since I joined the code review group (and even before), there have been a number of excellent suggestions for improvement to the existing process. In addition, there have been numerous discussions regarding the merits of various improvement options. While there has been some headway (especially in the areas of documentation), general inertia causes most of these to become no more than ‘lots of talk, and little action’.

Perhaps it is due to the do-ocracy of Drupal … and admittedly, frustration with this lack of action did inspire me to go chase after the automation of coder reviews for new project applications (still in progress). In any case, it’s my hope that putting forward a solid and defined proposal and roadmap will help inspire others to step forward, pick up one of the various components, and chip in to help with the ‘action’ component, without which, the ‘talk’ is meaningless.

Pain Point 2: Quality of Applications

Incomplete Applications

Occasionally, reviewers settle down to perform a review, and find that the applicant hasn’t taken the basic steps requested of the application process. Often, we’ll see project applications without sandbox links, with empty sandbox repositories, or with no description of what the module is actually supposed to do.

This was a significant issue for full project applications which were moved over from the CVS queue.

Whether the issue is one of applicants not reading the application process documentation, not understanding the application process documentation, not knowing about the application process documentation, or simply stumbling over the application queue without any previous knowledge of the process, incomplete applications serve as a significant de-motivator for application reviewers, who often need to wade through a number of incomplete or otherwise sub-standard applications before finding one which is actually ready for review.

While the code review team has put some significant effort into improving the project application documentation to try and reduce the number of incomplete applications appearing in the application queue, it will take some time to verify whether these changes actually reduce the frequency of incomplete application appearing in the queue.

Code Quality

The project application process has been positioned as a mentoring opportunity for new contributors, and in many aspects, it does serve well as an education tool for full project applicants. However, the process requires a finished module before applicants can submit their code for approval. In many cases, the code for the submitted project may be of an unacceptable quality (usually due to security issues). More often, the code is perfectly fine PHP code, but doesn’t take proper advantage of any of Drupal’s APIs.

In my opinion, treating the project application process (as it stands today) as a mentoring opportunity is better than having no mentoring at all … but in practice, this mentoring occurs too late in the development cycle to maximize the value-for-time efficiency equation. Instead, this mentoring relationship should begin at the start of the design/development stage to ensure that proper use of Drupal APIs is considered from the beginning of development, to reduce the potential for rework and simplify the review process.

Pain Point 3: Sandboxes

The move to sandboxes with the Git migration was an enormous step forward for new project development. Access to version control and issue queues during the development and testing stages of a project is invaluable to contrib module developers. However, I feel that the sandbox concept as it stands today falls just a little bit short.

Sandboxes Require Git

Yes, this was a conscious decision. We want to protect against the possibility of uneducated users stumbling across (and downloading) dangerous sandbox code. So as a result, the decision was made to not allow the creation of releases in project sandboxes.

However, contrib authors depend on feedback from their clients/testers to identify bugs in their pre-released projects. These clients and testers are typically end-users of the module functionality. Developers (and only some of them!) understand Git … but end users do not.

In creating a hurdle to protect the ‘stupid user’, we impose a testing barrier on the new contrib author. Most of their clients and testers (and even some of the developers themselves) have never actually used anything other than tarballs and zip files. In many cases, it’s the only approach they know … and even .tar.gz goes beyond the comfort zone of many testers, clients, and end-users.

By making it difficult for testers, we limit the amount of pre-release testing and feedback which can be solicited by the developer of a new sandbox project … as well as increasing the amount of time that developer ends up spending on support tasks, rather than the improvement of their module.

'Sandbox' Stigma

A side effect of the aforementioned decision is the perceived stigma which has been associated with sandboxes as a result. To a new contrib author, the perception is that their sandbox project isn’t a ‘real’ project until it is promoted to full project status.

Considering the significant delays present in the application queue, and contrasting the applicant’s experience with the fact that approved users can immediately create as many ‘full projects’ as they like, many new project applicants get the sense that they are being treated as second-class citizens. Some applicants have expressed that it felt they were being stonewalled and made to jump through hoops before being allowed to create ‘real’ projects, relative to others (already approved) who were actively producing loads of inferior products … and in some cases, projects which are direct competitors or duplicate modules relative to the applicant’s own project. (To add insult to injury, applicants would then be asked to explain why their module should be allowed in contrib, when 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!)

This stigma arises for a number of reasons … lack of release capability, lack of a project namespace, lack of press (though sandboxes do show up in regular search, just not under ‘modules’) and the ‘full’ project label itself.

Pain Point 4: Lack of Reviewers

While there is a significant lack of metrics available regarding the module review process, it’s safe to say that the existing process is not maintainable with the current number of active reviewers. Even if the current team was able to keep pace with the incoming applications (which is approximately correct), there are not enough reviewers available to clear the current application backlog without a significant sustained effort (and the corresponding burnout which would result).

There have been numerous discussions within the code review group regarding ways to bring in more reviewers. However, while the process allows for anyone to take on a module review, there has been a fair bit of concern suggesting that potential new reviewers feel intimidated by the process, or hesitate because they either don’t know how to review, or don’t feel qualified to perform a full review. There is also a chance that a sudden influx of new reviewers could lead to a lack of consistency in reviews.

Pain Point 5: Module Duplication

Module duplication is a growing concern within the Drupal community, which values joining forces on improving one project rather than building multiple competing modules with different features and overwhelming end users with choices. Frequently, projects come forward which would be better suited as enhancements or feature additions to existing contrib projects … or, in some cases, which are direct equivalents of existing projects.

As this is one of the only gates involved in the module contribution process, reviewers strive to ensure that new contrib authors understand the importance of collaboration, and searching for an existing solution before adding a new module to the contrib repositories. Next to timelines, debates regarding module duplication are a large source of frustration and conflict with project applicants during active reviews. My first exposure to the code review process happened during the middle of a disagreement between two reviewers regarding whether an application actually constituted ‘duplication’ of an existing module … which, unfortunately, served to kill the applicant’s spirit before they even got a foot in the door.

In a perfect world, the module duplication check would occur before the contributor has written a single line of code. This allows the opportunity to redirect a contributor towards a collaborative effort (building on an existing module) before they have invested any time or effort into creating their own solution. This helps to eliminate wasted effort … and, more importantly, takes place before the applicant develops a strong ‘emotional’ attachment to their own code/contribution/approach.

Of course, this ideal situation does not fit well within the Code Review process, which requires a ‘completed’ module/theme before an applicant can even submit their full project application.

Pain Point 6: Applicant Patience and Perceptions

As society trends more and more towards the concept of ‘instant gratification’, there will be more and more applicants for whom an answer tomorrow is two days too late. While little can be done to satisfy these folks, I have to sympathize with applicants who post an application and then receive absolutely no feedback for weeks at a time.

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. While being made to wait up to four weeks for an initial review, as well as for subsequent followup, this leaves a lot of time for applicants to grow impatient … especially when they realize that there are no hurdles, limitations, or reviews imposed on anyone who has successfully been approved; or when they see official modules in contrib duplicating the very same issues that they are being told are show-stoppers for their application.

This leads to an illusion of inequality … to applicants, there is a perceived double standard - the lax standard applied to those who happened to get in before the project application process was introduced, versus the much stricter standard applied to applicants who aren’t being allowed in later. This in turn leads to some applicants sensing they are being treated as ‘second class citizens’ within the community, which kills their contributing spirit/motivation.

Pain Point 7: "Project" Approval versus "Developer" Approval

While the full project application process is just one process, successful completion of this process means two things things:

  • The module is vetted as ‘ready for inclusion’ in the official contrib project lists, and can thus be promoted to an ‘official’ project with an ‘official’ namespace … in other words, it becomes a ‘real’ project.
  • The applicant is vetted as ‘ready to be trusted with the “promote projects” ability’, and is thus let loose to create whatever future official contrib modules projects they want, no longer subject to limitation or review … in other words, they are promoted up from ‘second class citizen’ for those applicants who might perceive it this way.

With the existing process, 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.

Pain Point 8: So close, but yet so far!

Occasionally, an applicant runs through the long, frustrating new project application process, waiting for reviews and addressing the issues found in those reviews … only to have a reviewer come along at the end and point out ‘this module duplicates X’, or ‘this module should be a feature request on Y’. Even if the contributor’s submission has successfully demonstrated adherence to best practices and knowledge of the Drupal APIs, the patience to listen and implement suggested improvements, and the dedication to stick with it through the entire frustrating code review process … a ‘module duplication’ claim essentially blocks the entire application. For that user to be granted the ‘promote projects’ ability, they must go back, develop a new module, and start over at square one of the project application process (and all of the associated waiting which goes along with it) … when their only real fault was not performing a thorough enough search before coding their project.

In my opinion, there is a need to separate the process of ‘getting a module review’ from the process of ‘getting a project promoted’ from the process of ‘granting access to create “real” projects directly’. In the situation described above, I don’t see any reason to block the ‘full project permissions’ role … having the module denied should be lesson enough for that user to not make the same mistake again.


This is part one of a four-part post. The rest of the proposal can be found at the following links:

Or, if you prefer, download a copy of the entire proposal in Word format.


Comments

One thing that concerns me is

davidhernandez's picture

One thing that concerns me is understanding who is responsible for a review. Some of the delay is caused by someone posting a comment, and everyone else assuming that person is handling the review. Many reviewers, including myself, skip over reviews with comments, because we assume it is being handled, and don't want to duplicate effort. This would also be a problem if we encourage newer people to participate in the review process by only checking certain things.

True

jthorson's picture

I used to do the same thing ... but the assumption that comments = review doesn't hold. I came across a four-month old application with 32 comments, which hadn't had even an initial review - instead, the reviewer was posting their updates to the application each time they changed the project.

Since then, I'll always set myself as 'assigned' if I'm involved in a particular review; and if noone is 'assigned', then I go into the comments to check whether it's being actively reviewed.

In any case, this proposal doesn't really change anything with the full review process ... today's process (and problems) would continue at the 'certification' stage. However, I think that the 'who is reviewing' issue is largely a factor of the enormous application backlog.

One of my core beliefs behind this proposal is that almost all contributors would like to be able to create releases, and most would like an official drupal.org namespace ... but only a % of contributors will actually attempt to achieve 'certified' status. As a result, the volume of 'certified' applications should initially be lower than what we see today. Combine this with the 'clean slate' approach (with regards to wiping out the backlog and allowing everyone to generate -dev releases), and you get a fresh queue with fewer applications; which in turn helps alleviate the 'hunt' associated with selecting a project to review.

Not that this proposal can accomplish it alone, but wouldn't it be nice to have a queue that was clean enough that 'duplication of effort' was no longer a concern? Especially when reviewing ... more eyes looking at the code isn't necessarily a bad thing!

Code Review for opt in security advisory coverage | Home

Group organizers

Group notifications

This group offers an RSS feed. Or subscribe to these personalized, sitewide feeds:

Hot content this week