Meta Discussion: Project Application Process Revamp

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

PROPOSAL SUMMARY:

  1. Initial Git access agreement and sandbox creation stays status quo.

  2. Develop automated testing capabilities for everything from coder to xss screening to scanning the repository to ensure it contains Drupal-like code.

  3. Run ALL modules through automated testing before allowing promotion to full projects

  4. If a user does not have 'create full project' permissions, then have them submit a 'Project Submission' to an issue queue.

  5. The issue queue is intended as a 'sanity check' only ... not a full technical review. The main issues should be addressed through automated testing. The main purpose is:

    • keep tabs on how well the automated tests are working,

    • provide an alternative path for modules that "can't" pass the automated tests (with valid reasons), and

    • provide corrective direction and/or suggestions for items which can't be caught via automation.

  6. Any issue in this queue which sits for two weeks in Active or CNR state, without new comments, automatically gets bumped to RTBC.

  7. Applicants which clear the queue get access to a 'promote' link for that project.

  8. Create full project' rights allow an individual the option of promoting any project which passes the automated testing directly, without applying via the queue (though they can still use the queue, if they so choose).

Optional discussion:

  1. Tie granting of 'create full project' rights to a competency demonstration (via a challenge quiz).



Revamping the Project Application Process

As many of you are aware, there have been a number of recent Project Application discussions occurring in the Drupal community, building on momentum kicked off at the related DrupalCon Core Conversation last month. The general consensus in these discussions has been that the current Project Application process is not meeting the needs of potential contributors and the Drupal Community, and that immediate action is required to help address and rectify this situation.

While these discussions have resulted in some excellent suggestions for improvement, many of these conversations have taken place primarily in person or on IRC. While these channels are great for individual debate and collaboration, they do not lend themselves to participation from the greater community at large ... therefore, a central repository. As a result, this page has been created to focus the conversation on a specific proposal for analysis and critique, and a starting point for tweaks and modifications ... with the hope that we can speed up the process, develop a consensus, document an actionable plan forward, and start executing on that plan without further delay.

So, as a starting point for further discussion, I'd like to offer the following proposal for your consideration.

I'll provide a flowchart of the full process proposal at the bottom of this article ... but for now, I'll break it into individual concepts in the interest of keeping things easily digestable.


Concept 1: Review Automation

In all of my discussions, this has been the one piece that everyone agrees on ... we need to automate as many of the review process checks as possible. The logical place to enforce these checks is just before the project is promoted to 'full project' status. I see these automated checks including the following:

1. Automated Coder Reviews (Must pass cleanly ... but at what Coder severity level? http://drupal.org/node/1299710)
2. Automated XSS Security Scans (http://drupal.org/node/786856)
3. Namespacing Review (ensure functions namespaced properly, verify project shortname uniqueness)
4. Licensing Review (scan for license.txt files, inclusion of external libraries ... how?)
5. Documentation Review (doc blocks, presence of README.txt, etc)
6. Repository Scan (repository contains code, key Drupal module files such as .info/.module present)
7. Demonstrated Basic Git Understanding (Maintainer has successfully created a -1.x branch, and -1.0 tag)
etc.

As the above checks would help to improve the overall quality of contrib projects in Code, I see them being applied against ALL sandboxes before promotion, not just those for new contributors. This brings up the question of non-typical projects with a valid reason for not passing one of the automated tests (or which trips false positives) ... which I will address in a bit.

For now, the 'typical' process for the majority of people (those who already have full project access) is outlined in the next image.


Concept 2: Users Without "Create Full Projects" Permissions

The above process is fine for existing contributors, but how do we handle the folks who do not have the "Create Full Projects" access? They do have the option of attempting the aforementioned challenge quiz, but making this mandatory would introduce a new (and signifcant!) barrier to entry for new contributors ... and we're trying to remove roadblocks, not put more up!

So instead, once a user's project is passing all of the automated tests, we have them create a ticket with their formal "Project Submission" notice in an issue queue. Here, human volunteers pick up the ticket and perform a sanity check on the following:

  1. Validate the automated test results look correct (ie. Yup! They all passed!)
  2. Ensure the sandbox project page has a reasonable description of the project (ie. Project Page contains more text than "this is my sandbox i'll add description later")
  3. Ensure the sandbox actually contains a project (ie. To discourage 'namespace squatting' and 'Just_for_Spam' projects)
  4. Ensure the applicant's chosen namespace makes sense for the project (eg. "mydrupalmodule" might pass a namespace uniqueness test, but let's keep it out of contrib)
  5. Ensure the applicant's chosen namespace doesn't conflict with existing projects (ie. avoid ending up with three different projects using the entityapi, entity_api, and entities_api namespaces)
  6. Ensure this isn't simply image slideshow module #374. (Allow 'duplicate' modules to encourage innovation ... these are intended as 'sanity checks, not 'justification points'!)
  7. Ensure the code doesn't include any external libraries.
  8. Ensure the git code repository isn't a total disaster.



Concept #3: Why keep the Queue?

  1. First of all, the queue name change is to reinforce that fact that this is no longer a 'project review' queue. The checks listed above are intended as quick sanity checks before rubber-stamping the application with an RTBC status.
  2. While it doesn't really lend itself as a full 'mentoring' opportunity, the human touch point does allow an opportunity to provide direction and correction for things the applicant may not be fully understanding, but which can not be caught via automated tests. (eg. "No, you don't need to create a new branch/tag after every commit!")
  3. Automated processes are great ... until you don't fit the assumptions they are built around, at which point they become a huge PITA. If a particular module gets blocked via the automated tests due to a testing bug or false positive, or contains code which falls outside the assumptions the tests are based on, then they can use the issue queue to explain the situation and request an exception ... allowing folks with 'full project access' an alternative when the automated testing says 'omgwtf? kthxbai!'.

So once the automated tests are in place, we mark all existing applications as 'postponed', with instructions to the applicants to re-apply once their module passes the automated testing requirements. (In fact, I'd suggest creating a new queue, and closing off the existing CVS and Project App queues entirely after directing existing applicants to the new process). And then we auto-prune to keep the queue from growing out of control again.


Concept 4: Application Auto-pruning

As long as the number of open issues is kept at a manageable level, and the action required to address an issue is relatively minor, than issue queues are actually very efficient. With the existing Project Application queue, however, the action required to make progress on an issue often required an hour or so on the part of the reviewer, and sometimes days of effort on the part of the applicant ... and most tickets needed to be revisited numerous times before being closed off. As a result, the number of open issues in the queue grew to the point where the reviewers could no longer keep up, and the sheer volume began to serve as a demotivator for people volunteering their time to perform reviews.

To prevent this scenario from re-occuring, there is a need to ensure that i) applications can be vetted and approved with a minimal time commitment, and ii) a process is put in place to prevent unattended applications from slipping through the cracks.

Moving from a 'review' process to a 'sanity check' helps to satisfy the first condition. To satisfy the second condition, any application which goes idle (no comments or updates) for two weeks, and has not been marked as 'needs work', 'postponed', or 'closed (won't fix)', is automatically granted an auto-RTBC.

The theory behind this is that auto-pruning the queue in this manner will help keep the queue size down to a manageable level, and the two-week delay is to provide a reasonable chance for reviewers to re-visit applications which were flagged as having potential issues, before the issue is pushed through (but still not permanently block said application when that re-visit doesn't happen).


Concept 5: Per-Project Promotion (without 'create full projects) permission)

The last piece of this puzzle that is required is the decoupling of the 'Promote Project' link from the 'Create Full Projects' permission ... instead enabling the ability to enable it on a per-project basis. When someone (who: require 'full project' rights? Git Admin?) promotes a Project Submission to 'fixed', this should trigger the appearance of the 'Promote Project' link on that project ... but the exact timing of when the project is promoted to official Contrib status should still be left in the control of the applicant/maintainer.


Concept 6: (Optional discussion) Separation of 'code approval' from 'developer approval'

One of my long-standing criticisms of the existing Project Application process is the fact approval was 'all-or-nothing', in that if your module didn't get approved then you didn't get approved ... or, alternatively, there was no way to approve a simple module without also giving the author free reign on contrib, even though the module may not contain enough code to demonstrate programming competence and knowledge of Drupal APIs/licencing/security requirements. One of the suggestions which came out of the post-DrupalCon discussions on this topic was the development of a 'quiz' to validate a user's knowledge of Drupal's APIs and common security issues. To me, this would serve as a great tool to validate the 'developer approval' side of this equation.

User passes 'Full Project Certification' quiz --> User is granted the 'Create Full Projects' permission


Putting it all together ...

Combining the above concepts/flowcharts gives us the following result ... a complex and convoluted flowchart, which should hopefully result in a much simpler process. Comments, opinions, and feedback encouraged!!!


Comments

I think the automated tests

sebasvdkamp's picture

I think the automated tests would be a big help. Also streamlining what to be on the look for would make it so everyone gets the same experience.

In order to pass a test you could use a transparent points system. An application needs x points to be accepted, if you've been reviewing projects since day one you're all it'll take if you're a new guy You'll need x others to confirm. This might take a bit of the weight off when you're in doubt about a module or an action you're about to take.

Not to sure about the auto RTBC, it wasn't reviewed, it isn't ready for whatever reason. Simply pushing it through because no one had the time? Do we really want that?

The concept looks good! Here's to hoping the application queue can get back to normal wait times ;)

Re: automated tests

Simple Rating System

ozwebguru's picture

Would it not be better if Drupal.org adopted a simple user rating system for the contrib modules by the community. Drupal.org only presents a list of all contrib collected from git hub subject to some config parameters. Drupal.org only provides tools like coder, devel, test suite, coding guidelines, style guide etc. Would it not be better organisation approaches in democratic manner than bureaucratic process. This would pave way to a real and open Drupal app store used and rated by the users.

Ratings

jthorson's picture

I believe that a combination of automated testing and user reviews would be excellent ... the above is a suggestion for a 'next step' towards that utopia, with the intent of reducing the amount of bureaucracy already inherent in the existing process.

Community Ratings ++

beanluc's picture

I totally agree, and I have made this point in a couple of other discussions.

d.o modules repository has the "how many sites" metric, which could be considered "votes for" the module, but why not add fivestar and text reviews. We've been talking about the "problem" of approving projects, but what hasn't been stated in this discussion is that no matter what the project-creation process is, end-users still have the problem of choosing modules. I can't think of any good argument against community ratings. With community ratings, who even cares if a crap project winds up getting created - it will be recognizable as such.

We tried "crowdsourcing" quality review, by means of the code-review process. That didn't work, but, it doesn't mean there's not another way to crowdsource quality review - a way which is far, far more organic, democratic, accessible to all (not just developers), and accessible forever (not just during the application process).

crap projects

beanluc's picture

"who even cares if a crap project winds up getting created" OK, that's a little strong. It's rhetorical. Please respond regarding the "community reviews" idea or the "improve project applications process" idea, and not to the "let's foster crap" idea, because that's not what the idea is.

The idea is restoring sanity and introducing accessibility to the project application process. If we happen to wind up with something which benefits end-users who are trying to evaluate module choices at the same time, so much the better for the Drupal project and the Drupal community.

Community Ratings ++ redux

beanluc's picture

With all this in mind, I'm going to take a stab at actually implementing this.

It turns out this is something which has been discussed for literally years now (like, at least 5) with enthusiasm. Greggles and webchick helped me to find:
http://drupal.org/node/50605
http://groups.drupal.org/node/7191
http://groups.drupal.org/node/80004
http://groups.drupal.org/node/86819
https://infrastructure.drupal.org/drupal.org-style-guide/prototype/modul...

I had no idea there was such pent-up demand for this. I came up with the idea just as a balm to soothe certain potential fears around eliminating certain elements of the code review and project application.

Namely: removing all bars to project-promotion approval (on a per-project basis, anyway) with the exception of eyeballs-on security checks and all the automated checks we've been talking about. (Maybe a couple of other sensible elements too which I'm neither remembering nor looking-up right this second.) "Code review", "mentorship", "avoiding duplication", and more, are all very positive community values and should be preserved - but in ways which don't impede and discourage contributors.

That might sound scary and trigger visions of a catastrophic decline in the overall level of quality in the projects collection. Like, to at least as low as the abysmal quality represented by the third-party contrib repositories of FLOSS communities like (insert names of any ten extremely popular and high-profile FLOSS projects here - CMS's, web-app frameworks, or otherwise). So I came up with the idea of replicating what many of those "other communities" do, with regard to getting the community involved with letting each other know which contrib projects rock hard (and which ones merely sort of drunkenly noodle, like an accordion player at a massive pipe-organ keybank).

My hope was that implementing this would mitigate any fear around the effect which "quickie approvals" might have on the contrib repository's reflection on the Drupal community. Yes, I recognize that dupes and cruft might wind up being promoted to full projects - but I violently believe that that does not have to harm Drupal, Drupal contrib, Drupal boosters, or Drupal users. Seems to me that community ratings and reviews are the secret to preserving and even enhancing the community oversight of the contrib repository's usefulness and reputation.

Speed...

markwk's picture

It took me 5+ months to get my project checked, then a month for "review," which involved basically "document better" and "document better the Drupal way." Definitely slowed me down from contributing earlier and better.

I like automated tests!

Acceptance and support are key

elseif's picture

I think that's a good process description and I've seen several development projects benefit from adopting automated testing, review and approval processes. Where I've seen them bog down is after they're set up. If they don't have good process documentation, response to feedback from the developer community and a policy of actively engaging people then the developers get frustrated, the people running the system get burned out and, as Jeremy said, it turns into a massive PITA.

The places where it did get implemented correctly didn't have to do that much extra really - they made sure they documented the process clearly (actually, this proposal article has a much better description than some of the production systems I've worked with), built in feedback mechanisms such as 'your project is here' displays and developer support such as ensuring that developers have access to up to date material required to pass the equivalent of the challenge quiz.

c4rl, sreynen and I discussed

greggles's picture

c4rl, sreynen and I discussed some of these ideas over dinner last week. One thing sreynen brought up is that automated reviews of projects will only reduce the effort to review the project by 10 or 20 percent.

Consider:
1. Automate check of code style
2. Automate check for license.txt
3. Automate check for README.txt

That's great, but in a typical review it only takes a few minutes to do those. The stats from the presentation in London talk about how 64% of projects don't pass this test, but how much time does it take to do this part of the review? I think we have to recognize that if our goal is to increase the speed through this queue then automating those things won't help much with making it faster. It WILL give applicants a better impression because they are getting the answer faster. FWIW, I've started using mywords to give more friendly boilerplate text in my responses for these and similar tests.

The real time:
1. Reviewing code: security, API issues

That one will always take a long time :(

The conversation with sreynen and c4rl happened before we'd seen these flowcharts. I think it's important to recognize that this proposal is not just about automating pieces the current process, but also about changing the flow of the current process. The proposal is to automate the review queue, remove the technical review in the project review queue, and create an alternate flow for a quiz only path to becoming a full project contributor.

The presentation in London that a lot of this is based on was created in person by people who had limited experience with the project application queue. It's true that as a team they reviewed 60 applications to get a sample of what has happened. But that queue has 800 issues in it now. I'm not sure covering 60 issues as a team really gives the background necessary to get a feel for the process.

The presentation also stated that robots are good at finding security issues. I disagree :/ It can certainly help to find security issues, but it's not really sufficient. I do think that the Quiz idea will help to get more depth and breadth into the security review portions

I think we should move forward automating the pieces of this that we can automate /and/ where people agree on the idea.

I am opposed to a drastic change in the workflow until we can improve the tools for evaluating modules.

Some more anecdotal stats:

Unrelated to all that, I just reviewed I just reviewed 8 applications that were in the RTBC queue.

Fixed:

  • jBart - Moved to fixed.
  • Honeytrap - Fixed though the install instructions could be a little better.
  • Talata fixed though I'm a little concerned. The author seems very interested in promoting his company in ways that feel out of line with Drupal community norms. I'm hopeful the process has been a useful learning experience about what is and isn't appropriate in terms of self promotion in themes.
  • Ubercart Global Quote - Fixed! It had some small issues remaining, but none that should hold back a project.

Not fixed, mostly small issues:

  • App store feature - Moved to needs work. Missing some basic explanation in the README and project page. It's also nearly impossible to review because it seems like pure exports of configurations. This is a case where a the quiz concept would be much more valuable than reviewing the feature because it probes a deeper understanding.
  • Github connect Moved back to needs work because it seems like it duplicates oauth connector. Also has has some very minor issues but those shouldn't block approval.

Not fixed, security issues:

  • Media Nivo Slider - Moved back to needs work for several issues including a security vulnerability that was not identified by secure_code_review nor coder.
  • Biblio Mendeley Connector - Moved to needs work because of a security issue and small things like LICENSE.txt and README.md. Secure_code_review nor coder would have identified this issue. I believe this is an example of a review that is more like the mentorship that was raised recently as a potential impact of the process.

A quiz that included security topics might have helped teach these people how to avoid these and other issues.

So of the 8 projects I reviewed that were already RTBC, 2 of them had newly identified security issues in them. 50% of them were truly ready to be fixed.

The presentation also stated

webchick's picture

The presentation also stated that robots are good at finding security issues. I disagree :/ It can certainly help to find security issues, but it's not really sufficient. I do think that the Quiz idea will help to get more depth and breadth into the security review portions

Well, I think the point is that an automated script that knows ! in t() is dangerous and flags a warning about it is more consistently reliable than a group of humans with uneven understanding of the inner workings of http://drupal.org/writing-secure-code. The fact that a member of the security team had to mark two issues that were RTBCed by the code review team down to needs work for security problems sort of proves the point the presentation was attempting to make. And while those automated tools might not catch ! in t() today that seems like a pretty solvable problem.

It's also worth noting that the process outlined by jthorson doesn't get rid of human review altogether; rather it moves it down to more of a "sanity check" because, with the improvement of automated tools, that's hopefully all it should be. No more wasting valuable human braincells on coding standards nit-picking or $variables directly in db_query(). Instead, all of that "easy" stuff is taken care of, and our valuable human braincells can be focused on actual mentorship, and it comes across to new contributors as actual mentorship and not frustrating nit-picking.

I personally really like the idea of a quiz, because that's how you get to the root of the education issues you're trying to solve. But it was pointed out that the very first thing people are going to try and do is game it. I don't personally care that much; even if someone cheated their way through, they at least had to read the question and select the right answer so maybe something got through. :) But it was contentious enough that it was left as an "optional" piece in the final round.

Present process needs to change

mlncn's picture

Greg, if we had 10 of you and some reasonable multiples of the others doing great work in the project review queue (and 500 of me for my impact on the queue to be perceptible) that would be great. But we don't right now, and so the process really does need to change. We need reduce the need for human involvement, and then build it back in as we can sustain it in the ways that it is clearly needed. This proposal does not stop any amount of human review we want and are able to put in, but it explicitly removes it as a bottleneck.

There are other (complementary) solutions. I'd be happy for a requirement for applicants to have to contribute to the review of someone else's application before their's is approved. (And my own long-standing proposal is that every project promotion need the approval of at least one other person who has had their projects promoted- a simple sanity check applied to existing and new contributors, though new contributors would likely have to go through the queue to find someone to approve their project. It would also require us trying to spread the tenets of code review to the full Drupal coding community, which would also be good in itself.)

The proposal above is solid and if it requires cutting boombatower a check, great. Regardless, something needs to change.

Right now months in the queue is simply a broken promise, from we the Drupal community to our newest members.

benjamin, agaric

I agree something needs to

greggles's picture

I agree something needs to change and I think I said that in my comment ;) I'm all for automating the things we can and, in fact, have done the majority of the work on writing content for the quiz (at least the security related portion of it).

But as you point out the human review part will be around and to satisfy the demand we will need more. I think we can grow 100 of me if we encourage people and keep them interested in the queue. From August 18th to now we lost 40% of the top 10 reviewers. And as we've seen from a dozen different top contributor power curves - the top 10 folks are the key folks to keep working. So, why is it that we lost them?

Anecdotally people have been saying that the process of encouraging those in the queue to review others has only resulted in jthorson getting super involved, but my research into the 40% yesterday helped me learn that's not nearly true. eosrei and ccardea were both some of our top reviewers and they got involved while waiting for their applications to be approved.

"lost" was the wrong word.

greggles's picture

"lost" was the wrong word. Obviously they are still around and doing things. And we always need to remember there is life outside Drupal which can get in the way... ;)

Maybe "what reduced their involvement and what can we do to reduce 40% monthly turnover" is more what I was trying to get across.

I think another question is

webchick's picture

I think another question is "What can we do to get the 500% people we lost back?"

I used to do code reviews back in the day, and really enjoy them. As did people Michelle (who I won't attempt to speak for). But when the process morphed into its current state, I pretty lost all motivation for helping, because it really felt like the focus shifted to why NOT to give someone access to make Drupal awesome, rather than a simple sanity check as it had been in the past. I can't get behind putting volunteer hours into telling people why they CAN'T contribute to Drupal.

If you move human review back to a simple sanity check and actual mentorship, which is the goal of this proposal, I think myself and others I'm sure would be happy to get back involved again.

I don't see how you can have

davidhernandez's picture

I don't see how you can have "simple sanity check" and "actual mentorship" at the same time. If the goal is to speed things up, and increase acceptance, that will mean less human involvement (which is fine.) It seems to me that mentoring might need to be removed from this process, and be its own initiative.

YES!

webchick's picture

I would LOVE us to de-couple mentorship from the actual process of granting access to stuff! This is a drum I have been beating for a long time. It would mean comments by people in the code review team would be viewed by new contributors as a help (wow, thanks for taking an interest in my project and for the great advice!), rather than a hindrance (why are you standing in the way of me giving back to Drupal by telling me my code has whitespace issues?). One "lo-fi" option would be a view of all commits by users who've had git access for < 3 months or something, or an "opt-in" checkbox on sandboxes that says "I would love someone to review my code." At the access part, we really only need be concerned about are security and license problems, not things like coding standards (though those help us in finding security problems).

However, what I meant by that more specifically (and what is outlined in the post above) is that if the following things are already done by the time it gets to the code review queue:

  • Coding standards are met
  • LICENSE.txt is removed
  • There are no external libraries
  • There are no $Id$ tags
  • There are no blatantly predictable security holes (e.g. db_query("Hello $world")), t('!dangerous'), etc.)
  • There are no blatantly obvious API problems (calling theme_X_Y() instead of theme('x_y'), function namespacing, etc.)
  • There's a Git tag/branch already made
  • etc.

Then suddenly 90% of what the code review team is currently spinning cycles on is gone, and volunteers can focus their time instead on providing the keen insight and advice that only humans can give. And a new contributor's first interaction with a human isn't a gigantic list of nit-picks that seems like "busy work" when all they want is to give their module away to us so people use it.

I don't think this is 90% of

davidhernandez's picture

I don't think this is 90% of what the code review team is currently spinning its wheels on. It might be 90% of what is commented on, but the issue comments don't reflect how much time a reviewer spends on what. I can post multiple comments on coding standards, $Id$ tags, blatant problems, etc, but that might only be 5 minutes of work. One small comment on a bug might have taken an hour reading code and playing with a module to realize.

Sure, that's fair. But the

webchick's picture

Sure, that's fair. But the substance of those comments are really important. Realize that to an applicant, when they see a review that is 90% coding standards-focused and 10% actual bugs, they can feel picked on and frustrated. Especially when it might be another 3-4 weeks before they hear back from a human again.

The fact still remains that if all of that nit-picky stuff was done for you before you looked at the code and did a review, you would catch a lot more substantive problems with the module because your eyeball wouldn't be twitching about that incorrect whitespace (or at least that's what my eyeball always does when I see incorrect whitespace :D).

The same thing applies to core queue, as well. I've seen people get completely burnt out on core development because they sit around waiting for a review from a human on their patch, and then when they finally get one it's talking about whitespace and the proper way to write @param in a comment docblock. What people are hoping for instead in those reviews is substantive feedback about logic problems, ways to make the code more efficient, and things like that. Those kinds of comments are invaluable. So let's let robots take care of the easy stuff, so we can focus on the hard stuff. No one's really going to fault a robot for nit-picking; that's what they're good at. ;)

The best part of this proposal is that once this is implemented for sandboxes, we can also use it across all projects, including core. And suddenly the collective IQ of the development community about coding standards and security issues goes up, and the time our code reviewers are spending gets laser-focused into the really valuable feedback that people are eager to hear.

The issue queue is intended

davidhernandez's picture

"5. The issue queue is intended as a 'sanity check' only ... not a full technical review."

The proposal, in Concept 2, also states that sanity checks should only be worked on for <30 seconds. There is no room in this proposal for really valuable feedback, or in depth analysis. If that is the plan we're going with, lowering the bar for entry and prioritizing acceptance over analysis, that's fine. There are pros and cons of both. I just want to make sure the level of human involvement (and workflow) is clear, because there seem to be conflicting messages.

make sure the level of human involvement [...] is clear

beanluc's picture

Hear, hear, that's pretty much what I meant down here too. Just that we need to be really clear - ideally be even more clear about the intentions behind the process than about how the process itself should be spelled out.

Realize that to an applicant,

duckzland's picture

Realize that to an applicant, when they see a review that is 90% coding standards-focused and 10% actual bugs,

Jackpot!!!, Sometimes I wonder are we contributing a novel or modules code?

Some of the reviewer even bugging non english native speaker for a grammar error in his coding comments LOL.

IMO how can you really hunt for bugs when reviewer doesn't even try to install the modules and play around with it?

Peer reviews

sreynen's picture

davidhernandez, you might be aware of this, but I'm guessing many of us aren't, and it seems worth mentioning here: there's already a separate initiative that does mentoring outside this process. The Peer Review group has established a simple process for anyone to request and anyone else to offer reviews of projects, including sandboxes.

Unfortunately, almost no one uses that. In the last 8 months, there have been only 2 review requests, and both are still open. I suspect that's mostly because almost no one knows about this initiative. If we could figure out a way to increase awareness of the peer review option and make that a working mentorship experience, especially among new sandbox creators, we could effectively split the two and make both better. Mentorship would be more fun, because it wouldn't be tied to access, and application reviews for full project access would go smoother with prior mentoring.

One idea to make this more

webchick's picture

One idea to make this more visible would be a patch to drupalorg_project module (part of Drupal.org customizations) which could add a link to "Request peer review" in the right sidebar of a project page, which maybe links to a page callback that automatically generates an issue in the queue with the proper tag.

See http://drupal.org/node/1293124 for the whole song and dance about getting changes onto d.o, but http://drupal.org/project/drupalorg_testing could get you a long way.

Nope; never heard of it.

davidhernandez's picture

Nope; never heard of it. Joining now.

"we" not "you"

sreynen's picture

Nobody's telling anyone they can't contribute to Drupal. I've never seen that; if you have, please point to it so we can address it. I'd be shocked if anyone involved in this would think that's okay.

What we are telling people is that they can't promote their contributions to full projects (though they can continue to offer them as sandbox projects) until they've met some established criteria. And we're trying to help them meet that criteria. Few of us involved now were involved when that criteria was established, and many of us don't like it, so we're openly discussing ways to improve it, generally something closer to "simple sanity check and actual mentorship."

The key word there is "openly." There's no "you" here, only "we." If you don't see yourself as part of these discussions because we're not moving fast enough or whatever, that's certainly a fair choice; but to be clear, it's your choice, not anyone else's.

Scott. :( I meant "you" as in

webchick's picture

Scott. :( I meant "you" as in "one; anyone; people in general" not YOU you as in "the code review team" or "greggles." But the way you (and this time I do mean you you) are talking to me certainly makes me feel like an outsider in these discussions, despite having put hundreds of hours of work into policy discussions, infrastructure improvements, documentation and of course this very proposal, all in the interest of improving this process for everyone involved, including the code review team. I genuinely do not understand your hostility.

No hostility intended

sreynen's picture

Sorry, I'm not trying to be hostile. Much the opposite, I hope you'll continue to stay involved in these conversations, which is why it bothered me when it sounded like you were giving up and leaving the problems for someone else ("you") to deal with. I'm glad I only misinterpreted that.

Whew! Ok. :) Let's make sure

webchick's picture

Whew! Ok. :) Let's make sure we get a hug in at BADCamp, ok? :D

:)

sreynen's picture

Sounds good.

By 500% you mean the folks

greggles's picture

By 500% you mean the folks from the cvsapplication and prior days? I think most of those folks are in this comment and I agree some of them have overlap with current lists and some have left for known reasons (e.g. zzolo prefers to focus on coordinating bigger picture issues rather than doing reviews). But yeah, why are some of those folks who are still involved in the project not involved in reviews?

Gradual change

sreynen's picture

I am opposed to a drastic change in the workflow until we can improve the tools for evaluating modules.

I'm generally against drastic change in the workflow at all. I'm very much for changes, but I think it's worth taking it slower, one change at a time, to better understand the impacts of the changes we're making.

Almost none of the changes we're discussing here are inter-dependent. We can implement one change, give it a few weeks, see how it actually works in practice relative to how we expected it to work, and then make more changes with more knowledge. There are a lot of problems with the current process, but there are also a lot of benefits. Both the problems and benefits are complex, and we don't really know much about what we're doing. Even if the research for the London presentation were representative, it only looked at a very small set of relevant questions. By changing things too quickly, we risk both losing benefits and not actually solving problems.

To get more specific, when the quiz is ready, I suggest we implement it without changing anything else, give it a few weeks of people using it, request some feedback from people who took the quiz, look at actual quiz results, and make further changes based on what we see. If the quiz looks like it's fixing so much that reviews seem completely unnecessary, great. If the quiz looks like it's not actually solving the problem we expected it to solve, we'll be glad we didn't change more.

Same with Coder: when automated Coder is ready, I suggest we implement it without changing anything else, give it a few weeks of people using it, request some feedback from both applicants and reviewers, look at the Coder results, and make further changes based on what we see.

Now that I've written this out, it occurs to me there's a name for this kind of controlled testing of hypotheses: science. I've heard good things about it. :)

Wholeheartedly agree.

jthorson's picture

I had a similar comment in the first draft of this proposal which managed to fall out during the initial screenings and wordsmithing ... but perhaps I should have left it in ...

I'd like to draw attention to a key consideration in the drafting of this approach ... the natural human tendency is to over-correct, but too much change means more work, confusion, and retraining required. Also, once restrictions are removed, it becomes much harder to re-instate them again. This proposal attempts to strike a balance, prefering 'lots of small changes' to 'one big change' ... but consider it my view of the ideal 'next-state', rather than an ideal 'end-state'.

Of course, the proposal has changed a bit since I wrote that ... and while I think some change is needed, I also believe that 'measured steps' is the only sensible way forward.

Great initiative. Fantastic

giorgio79's picture

Great initiative. Fantastic writeup and effort jthorson.

Automation is key. I also understand those pushing for user code reviews, but it seems the effort and the resource does not meet the volume. I do sympathise with the peptalk that human review takes minutes, still it is not happening. I dont do it since at the moment I must put paid work in front.

Projects could get badges, such as reviewed by sec team etc etc. Or a warning could be shown for newbies, that this project was not tested for security etc.

Recently I started contributing to Wordpress, and it was a piece of cake. There were no human reviews. I just entered the name of my project, and I could contribute right away and upload via their svn.

****Me and Drupal :)****
Clickbank IPN - Sell online or create a membership site with the largest affiliate network!
Review Critical - One of my sites

New status name needed

beanluc's picture

"Auto-RTBC"? Really? heh

If no member of any community reviewed and tested a project candidate,then, the status "Reviewed and tested by the community" would be a falsehood. A semantically important one, not just a "little funny".

This isn't a knock against the proposal, it just is a suggestion to let RTBC represent what it represents, and to represent a status which isn't RTBC by some other name.

RTBA/B/C (automaton/bot/computer)?
"Passed automated checks"?

Does anyone fear how this would impact gitmasters?

beanluc's picture

If the intent is to automatically assign a status which is intended to signal gitmasters to go ahead and enable the promotion feature, does anyone think this is likely to slam the gitmasters with a pile of things which they are going to feel obligated to either review themselves or else ignore out of volume frustration?

Not to be cynical, but, I predict the volume of "auto-RTB*" promotions will be a very, very substantial proportion of project submissions. So what will gitmasters feel their obligation is, regarding that status? What do the rest of us feel the gitmasters' obligation is, here?

I mean: 2 weeks previously, a submission "needed review", nobody reviewed it, and now it no longer needs review? I echo sebasvdkamp, who said "Not to sure about the auto RTBC, it wasn't reviewed, it isn't ready for whatever reason. Simply pushing it through because no one had the time? Do we really want that?"

Just asking!

I suppose that my own opinion is to define that the "auto-pass" status means - yes, enable the promotion feature. If this is what was collectively intended all along, I missed it, so, I'm just spelling it out - let's not fail to be really explicit about defining our collective expectation of the process, before beginning anything like this.

Will "RTBC" even be used much anymore?

beanluc's picture

I predict the volume of "auto-RTB*" promotions will be a very, very substantial proportion of project submissions.

Since any project could more-or-less automatically be promotable by the contributor within a little more than 2 weeks of submission, doesn't it seem likely that human reviews will actually just dwindle away to nothing? Since no human input will be necessary to earn promotability, will any humans be motivated to show up and conduct reviews?

Now: I do realize that streamlining the process is intended to take the load off human reviewers, so that we'd stop feeling overwhelmed at the backlog and get productive again.

Just asking - does anyone imagine an unintended consequence here?

Definately

jthorson's picture

I don't personally see this occuring.

  1. With a smaller queue, it becomes a lot easier to motivate people to help maintain it. The 'auto-pruning' helps to keep the queue small enough that the end is always in site, with 'clearing the queue' never more than a single sprint away.

  2. With automated testing, the simple act of maintaining it becomes easier; those who are looking after the queue will be much more efficient.

  3. Two weeks is still a long time for a contributor to wait, and I anticipate applicants will continue to seek out reviews, in order to try and accelerate the process. (The difference today is that these applicants are typically shot down with a friendly 'patience ... your time will come' response, due to the sheer volume of today's queue.)

  4. The two week period is for those applications which fall through the cracks, and don't receive any comments in that timeframe. If there is something wrong, and an application is set to 'needs work', the applicant still needs to address those deficiencies - simply waiting two weeks won't help. On the other hand, if for two weeks, noone who looks at the queue has any comments regarding that particular application ... then what are the chances that someone will after four weeks? Six weeks? Eight weeks? Twenty?

The key is that we have to stop thinking of the queue in terms of what it is today ... Instead of a means for an applicant to 'seek approval', I see it as a time buffer for projects, to give the community an opportunity to evaluate and offer advice before a project graduates to contrib. The starting assumption is that the project will be promoted to contrib, and for this to change, it's up to the community to step up and provide a significant justification ... contrasted with today's "project will not be promoted unless someone goes out of their way to do so" model.

Can't we follow google/apple/amazon

dgoutam's picture

Lets promote all the project after automated testing pass. (Google accept all URLS to be crawled and indexed)

Lets the search do the rest for the users who are going to use it .. (Google returns the best results in a ranked manner)

Craps gets a residue rank and what wrong if the creator only uses the crap ..i do not think that community will be having any problem (No body goes beyond page 10 of google search results)

Have your facets ready to help users who needs help of results .. (In our case older with user reviews are good ..)

Have D.O suggested top modules / popular modules etc with stats to help more .. (apple )

I personally believe in open culture and found from my experience that users know the best to choose one , accumulate stats and purge the craps if want to.

So say we all.

stovak's picture

So say we all.

Require authored commits

eosrei's picture

I recently went through a multi-month application review process. It was discouraging, but finally set to fixed. While I waited, I spent some time reviewing other applications. A common problem was a lack of understanding about the Drupal.org community in general. So, I suggest we raise the bar to project application issue creation by requiring authored commits to existing projects. A users's first authored commit represents significant effort and study. After a few commits, they'll know far more about Drupal and drupal.org.

A few (2? 3? 10?) authored commits would:

  • be simple to automatically check
  • get more people working on patches
  • convince maintainers to use the --author git attribute
  • reduce the number of submitted project applications
  • hopefully increase the project application quality
  • spread the load of training new contributors across the entire community, rather than just reviewers

Motivation analysis and possible conclusions

doitDave's picture

I am impressed by the facettes such a discussion may reveal. :) Many good points in here, without doubt. I would like to hook into the process at a point slightly before it, after all, begins and that is "why would someone apply for full project permissions at all".

I moved the rest into a separate thread as it only partly relates to this topic here. Find it now in http://groups.drupal.org/node/195768

Code Review of Full Project Applications | Home

Group organizers

Group notifications

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

Hot content this week