Module Approval Process is Too Slow

We encourage users to post events happening in the community to the community events group on https://www.drupal.org.
You are viewing a wiki page. You are welcome to join the group and then edit it. Be bold!

We have many individuals waiting to be reviewed, and few reviewers. This gives us a backlog of applications awaiting review, which makes both reviewers and applicants unhappy. So how can we improve this?

Problem

The problem is that it does limit the ability to have application developers to get their code out. The system is logically sound, but is impractical due to resources. If we had enough code reviewers where the average turn around on a response from the issue query was a day or less the system would be great. If the system took 1 week for a module to get published, there would be no problem.

History

The review process has gradually evolved over the years to become more formals and in may ways more strict.

How the Review Process Works Now

New contributors create projects in a sandbox. When they want to move something to a full project, they open an application and wait for feedback from reviewers. Automated reviews pick up many common errors, but manual reviews are still necessary to catch other errors, especially security errors.

Suggested Changes

Get more reviewers How do we do this?

Make approval easier How do we do this without negatively impacting the security team (who would have more issues to deal with) and/or end users (who would see lower quality projects)?

Make reviewing easier If each review took less time, the same number of people could do more reviews. How can we make reviews easier?

Increase quality of projects before review Giving sandbox owners more resources to prepare for a review may speed up reviews.

References:
http://drupal.org/project/projectapplications
http://drupal.org/project/issues/projectapplications?categories=All
http://drupal.org/node/1011698
http://drupal.org/node/713102
http://drupal.org/node/645470

Comments

Thanks for not letting this be ignored

mlncn's picture

Everyone agrees the status quo is unacceptable- even with sandbox projects, weeks to months to get a review is not reasonable. Any process that applies only to new people is almost inevitably going to slide towards terrible. Requiring any trusted member of the community other than oneself to promote a project to full project status is not onerous-- Eaton can find someone to sign off on his next novelty module. A new user would have to convince someone that they will be a conscientious module maintainer. The key is that the someones not be a small group of volunteers, but, say, every Drupalista with three modules of their own in good standing.

To put my time where my mouth is, i'm applying for git-promoter powers.

I will then link from the Peer Review group to the project applications queue and the project expectations and coding-standards pages. (I'd appreciate someone giving me the contents of the old CVS Application Guidelines - http://drupal.org/node/59 - because if i remember correctly it was a pretty awesome code review process.) Not every review will be sufficient, i'm sure, but applicants will start to get more immediate feedback.

And i will not promote any project to a full project that has not been reviewed by someone other than me, but i won't necessarily let any objection from additional reviewers put a hold on it either.

Long term, perhaps paired with the "any peer review" promotion idea, i agree with Catch and Heine that putting a solid review before the first stable release is allowed makes the most sense, in terms of keeping a flood of 'supported release' modules from drowning the security team.

UPDATE: Permission to promote other people's projects to full project status requested!

benjamin, agaric

In the current system; The

MGParisi's picture

In the current system; The Peer review is only necessary for your FIRST module. Once you get that module through then your free to make as many modules as possible. In fact the peer review is so impassible that if you want to publish your module, you would be better to grab onto a module with no maintainer or become a co-maintainer.

Your example with Eaton is a perfect example. Eaton would not need someone to give him access to publish new modules, he already has the ability. I think I have the ability to create modules too, and I have long since forgotten about coding standards.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

mlncn's picture

And to be clear, i am proposing a change to the current system-- same rules for new and veteran contributors (no self-promotion, heh), except that someone who knows lots of other veteran people in the community will be able to get someone else to promote their project easily, while a new/unknown contributor will have use for a more formal review process-- but knowing who has the power to promote their project (unlike now) and having that be many people (unlike now).

Despite disagreeing with the present system, i will be trying to work within it to make it workable for new contributors.

benjamin, agaric

I want LEADERS!

MGParisi's picture

I want a republic, not a democracy. If everyone gets a vote, then the debate continues endlessly and nothing gets done. Appoint a group of active individuals who are willing to do the work, and have them do it. Remove barriers and give power. Add people as we loose people, If the issue queue gets short, then spend more time with people, if it gets long, get more lax. In the end, just get the job done.

Though on a project like this, I fear you will never have enough people. I can just point to documentation queue currently we have 6 pages (50 per page) 300 issues. Documentation seems fun compared to reviewing modules.

At this point, and why I feel so strongly about this that I am willing to push hard, even though doing so may risk my fragile status within Drupal, is that in this ONE case, this ONE area, we are treading water and going no where, and we will sink from exhaustion. I hear this exhaustion in people on IRC. We want leadership, we may not like what our leaders do, but we just want the go ahead or the no go.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Let the community regulate itself

kbell's picture

Putting community-moderated barriers up to prevent contrib modules making it into the system will kill us - I agree - for all the reasons stated above. This community seems to be really good at being self-regulating, and I have faith this will continue to be the case, as long as the basic freedom to contribute - no matter who you are, and without barriers - is preserved. When people use your module and it sucks, other people hear about it very quickly and the problem takes care of itself. Same goes for great modules, by and large. Developers who have a great idea but perhaps need some pointers in doing things "the Drupal Way" are very likely to attract mentorship simply by the fact of them making the effort to offer a useful module on drupal.org.

The coding standards and facilitating communication within the Community is what will continue to keep the quality high on drupal.org, IMHO. It's intimidating enough starting out as a contributor -- for those that take the plunge, leaving them sitting in a queue for 6 months before their work ever sees the light of day is just nothing but a disaster. That same module could be improving and its maintainer learning all kinds of new skills during that wasted time.

I think it's always best to err on the side of openness and freedom - have faith in users to demonstrate their critical thinking skills and in developers' desire to write good code, and I think we're always going to come out ahead

--Kelly Bell
Gotham City Drupal
twitter: @kbell | @gothamdrupal
http://drupal.org/user/293443

Let the community regulate itself

ccardea's picture

There was some clear guidance on this issue from webchick a few months ago

There will be an approval process of some kind, similar to the existing CVS account approval process, but far less daunting and time consuming, because:

* It will focus only on "big impact" items: overall understanding of Drupal's APIs, security review, license compliance, blatant duplication of other projects.
* Reviewers can use standard tools like commit logs, diff, and the issue queue to understand the code and its evolution and get a "sense" of the maintainer, thus removing a lot of pain and guesswork from the approval process.
* All non-critical follow-ups (coding standards compliance, naming conventions, etc.) can be handled in the standard issue queue way, and addressed by the author either prior to or after their sandbox makes the transition to a regular project.

It's clear that one of the goals is to make the process less daunting and time consuming. But please take note that overall understanding of Drupal's API's is considered a big impact issue, especially since once approved, you're approved forever.

Current code review process, and old CVS text

bonobo's picture

A couple resources here:

A wiki page on community code reviews - we will actually be doing one of these at the Portland Users Group in May.

And, as you asked, the old CVS application text (just cut and pasted here):

  • New translation projects are not hosted anymore on drupal.org; translations are done on localize.drupal.org and don't need a CVS account.
  • You do not need a CVS account to create patches for existing modules or to checkout core or contributed modules from CVS.
  • Read and understand the following before applying:

    • New CVS account applications go through an approval process. Our volunteers should be able to look at your application within a week, but this may take longer, so please be patient with us. The approval of your application can be delayed or prevented if the guidelines below are not followed or refinements are required.
    • Your work must be licensed under the same license as Drupal, which is GPL version 2 or later (read why). Also, we do not allow committing third-party libraries to CVS, even if they are GPL (read why).
    • Before submitting an application, be sure you've read and understood this page as well as CVS applications review, what to expect (which also lists reasons an application may be declined).

    For new projects:

    • Your new project must not duplicate the work done in existing projects. Instead consider collaborating with existing maintainers.
    • You must submit a finished, working module or theme for review along with your application. Upload an archive containing the code to review in the issue automatically created for you by the application. Our review will check for adherence to security best practices, usage of Drupal APIs, and coding standards compliance.
    • You must provide a motivation message, which should be a few paragraphs instead of only a few sentences. For themes, it should include also a screenshot of the theme (at least 640x400 pixels), and (when possible) a link to a working demo site; for modules, it should include also a comparison with the existing solutions.

    For co-maintaining existing projects:

    • Create an issue in the module's issue queue requesting co-maintainership. You should only apply for a CVS account once the current maintainer has already accepted you as co-maintainer.
    • Include a link to the issue requesting co-maintainership in the motivation message of your CVS application. Even if the current maintainer has asked you to become a co-maintainer, they still need to make a public record of this in the module's issue queue.

    Failure to read and follow this procedure can result in your application being marked as won't fix.

    The use of the Drupal Contributions Repository (DCR) is regulated by the Drupal Contributions Repository - Terms of services (TOS). By using the DCR, you are accepting, and agreeing to the TOS.

Gave Up

bkelly's picture

After trying to get involved for a while I gave up.

My company keeps me way too busy for me to spend hours and hours trying to contribute. If it was easy I might spend hours and hours contributing but I'm reticent to spend hours begging somebody to let me give them something.

It probably says something less than positive about me, but, I'm just not that nice a guy.

Those who will not reason, are bigots.
Those who can not, are fools.
Those who dare not, are slaves. - Lord Byron

Getting your teeth pulled.

MGParisi's picture

I gave it allot of thought and I fixed my post to reflect some changes to the document, but I disagree with the following

Minimal Review Process

- Readme - Should this be a requirement? I (mlncn) have plenty of modules without a README.txt, they are not easy to maintain-- though a README.txt with a link to the project page or other documentation at the least would be a very good idea.
- Version - what does this mean? Version in the .info is another thing added by the packaging script.
- License.txt should NOT be present- it is added by the packaging script automatically, and anyone with a sandbox has agreed to the GPL
- basic code review. Someone should look at it. Someone should try it out. We should not be nitpicky at all but with sandboxes completely open we should be looking for a minimum level of functionality and quality.

When you have 2,500 contributors willing to give away code for free, and you don't have the people to review that number, then we cant expect much of a level of quality. Now maybe I am wrong, and I think people should be able to talk about it. But installing and reviewing code seems an awfully hard thing to sell for people to do. Quite frankly it sounds like doing documentation at the dentist while getting your teeth pulled. But hopefully I am wrong and others enjoy this.

Couple that with the fact that right now you need administration level permissions and I dont see a choice but to open it up to all. Now maybe if we can get the permissions set differently then we could explore other options. But until then, the question we must ask is What percentage of loss are we willing to absorb?


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Enthusiasm over Quality

MGParisi's picture

I have noticed something in other social environments. We run a gaming group, and pretty much accept anyone who fills out an application then comes into vent and participates. Its amazing how many applications we get that do not even have the energy to sign into vent and say hi! So maybe the code doesn't matter but the level of enthusiasm does.

So can we agree that if people come on IRC and they pitch their module after filling out the issue, then they pretty much have done more then most will and we should accept these while we continue this debate?


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

"So can we agree that if

davidhernandez's picture

"So can we agree that if people come on IRC..."

Not to jump on you, but I just have one thing to vent about. I am really against people using IRC as the sanctioned forum for everything. I don't think it is appropriate. Most people don't use IRC, and most people don't want to use IRC. It is too real time. You have to hope you are in the right channel and the right people happen to be there at the right time. It is almost cliquish at times. It is great for real time chats with people you know, or getting help, but decision making shouldn't be done there, because it limits the conversation to only those who happen to be logged in an paying attention at that exact moment.

I'll jump on you! Your

Morbus Iff's picture

I'll jump on you!

Your arguments against IRC hold no real sanction with me: discussing things on a forum (such as GDO) have pretty much the exact same limitations of IRC: forums are just as "real time" as anything else, they just take more real time then IRC. A decision that might be addressed in IRC in an hour might take days over a forum and, with replies coming in at any time of the day or night, a discussant has to be a participant every day, versus just during a scheduled meeting on IRC. A decision is made in a forum on Thursday, after someone posited a solution on Wednesday, and I've been sick since Tuesday? Still pretty much screwed, just like IRC.

What you /should/ have said to support your point is that forums have greater "cohesive thought" than IRC (which is more "stream of thought diarrhea") because they slow down the conversation and stretch it out over a greater time, with better logging/catch-up-ability than IRC.

Ok. I find that forums have

davidhernandez's picture

Ok. I find that forums have greater "cohesive thought" than IRC (which is more "stream of thought diarrhea") because they slow down the conversation and stretch it out over a greater time, with better logging/catch-up-ability than IRC. ;-)

Irrelevant

MGParisi's picture

Wow, do we really have to hijack the conversation over which type of communication is better... lol. I don't care if he writes a letter, puts it in an envelop and sends it third rate USPS. That person has done something that 95% or more of our applicants dont. They show encouragement. I didn't think that statement was so vague as to start a flame war over IRC verse Forums... And No, I don't want to have a conversation about how people will snail mail me their letters from foreign countries.

And yes, my Dad CAN beat up your Dad :-p


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

"And No, I don't want to have

davidhernandez's picture

"And No, I don't want to have a conversation about how people will snail mail me their letters..." Aww, that was gunna be my next question. :(

My 5 cents

fuzzy76's picture

As a fairly fresh module contributor (approved last fall), I have to say that I fully agree with MGParisi. My CVS application took nearly 3 months to get approved. And my colleague had to wait nearly the same time to have his CVS access approved.

User ratings should help immensely in this regard.

I agree that applying

davidhernandez's picture

I agree that applying different rules to new people tastes a little funny, especially if it really is this hard. It is basically encouraging people to not participate. Personally, I don't have any public modules on drupal.org, because must of my coding is custom work for my employer, but I was thinking of releasing one or two modules I'm working on. I had no idea the approval process would take months. It almost makes me think why bother.

On the other hand, I do understand the need for drupal.org to maintain some kind of integrity. Being flooded with garbage modules, especially if they show up in search, can be problematic. It makes drupal look bad, and makes it harder for people to find what they need. Experienced users might be good at filtering through the junk, but newer people are not.

I'm liking this idea of making it easier to get approved, but in a dev state. This would let people get their stuff out there, especially if they are trying to share it with someone, and you could filter them out of searches. Then, once it is peer reviewed it can be fully released.

If every person who went

Bastlynn's picture

If every person who went through the application process felt an obligation or motivation to review at least one other person, to pass it forward so to speak, I think that might dig into the list by a solid chunk and be self-maintaining.

My own review took quite some time - my starting module wasn't business orientated, or particularly fancy on the code end - just a bit of fun - so it was hard to keep review interest. Eventually it went through due to a contact in the world who carried it the last bit to full approval. It was frustrating, but I'm patient and hard to rattle so I wasn't going to abandon the effort. I think it's reasonable to figure that most new developers aren't going to be like that.

Despite this frustration, I really do think we need some initial review. Other projects in the PHP realm that have a lower initial bar have been subject to collecting loads of frankly poor code. I'm specifically looking at the example of http://www.phpclasses.org I occasionally find gems there, but it's always after a lot of frustrating sifting. For the community reputation as a whole, we need to keep things to a standard.

As a second benefit to the review process, there's a certain degree of instruction and tutoring that comes with it for new coders. I learned from my reviewers when I was sending code through review. It got me on the right track with future code and gave me a clear idea of how to appraise code aside from just mechanically going through a documentation checklist. That's not an opportunity to pass up as far as I'm concerned. It's the first, most obvious, and probably best location to get a new contributor on board with the process and thinking about how their code sits within the system in the right way.

So, keeping the review process - I would like to see some ways to encourage a pass-it-forward response from those who already have an account. I would like to see "I mentored/reviewed X number of new accounts (who themselves reviewed X many more accounts)" thrown around with the same pride as "I contributed X number of modules or commits". (More than just saying you reviewed projects. I'd like some metrics - maybe a 'family tree' of accounts you personally helped.)

It might be worthwhile to track review and approvals on profiles the same way we track commits as a means of both recording and encouraging that initial mentorship? Doing so would also allow us to identify habitual reviewers for further nurturing.

edited: to clarify why I feel the current 'I reviewed Projects' account toggle isn't enough.

"I'm specifically looking at

davidhernandez's picture

"I'm specifically looking at the example of http://www.phpclasses.org..." - I second that.

With current system, how many people peer review your module? Is it assigned to a person or group of people, or is it just in a queue for people to look at if they feel like, with no guarantee that the same people work on it from start to finish?

If I recall right, it's just

Bastlynn's picture

If I recall right, it's just a queue with no guarantee a reviewer will be with you from start to finish - my review went through three different people. Which might not be a bad approach given that we can't be sure if the first reviewer won't come down with the flu or some other obligation mid review. Creating a formal assignment situation would obligate that assignment system to be monitored to avoid orphans.

"...to avoid orphans." That

davidhernandez's picture

"...to avoid orphans." That is what concerns me. I don't want to get lost, or see other people get lost. It is tricky. On one hand it is good to have at least one person there from start to finish, to help as things progress and to be a contact of sorts. On the other, you don't want to be stuck with a person that isn't helpful and worse, might hold back your progress.

That would be a concern for

Bastlynn's picture

That would be a concern for me too if the approval system were to move to an assignment system. I've worked a number of volunteer projects (editing articles in a queue) and in my experience assignment systems really need a lot of dedicated overwatch or a collection of OCD freaks. So I don't think that's a viable solution as we just don't have the time form review heads nor the population of OCD freaks to take either approach.

Rereading the original post - I think getting the metrics and posting display first will be the best next-steps we can take. I don't think the current review system has failed, so much as not been trained into the community mindset as it should be. "If you build it, they will come" doesn't work on volunteer systems. Just building the tool set to manage the information isn't enough but that seems to be the current approach to getting the community to use the project approval queue. If that isn't the approach being used, then that I'm not aware of the approach being used indicates we still have a problem in awareness. :-/

I think getting metrics and displaying that with the profile, gets the project approval queue some awareness, as an expected and key aspect of being on d.o. By raising the prominence of contribution in that area - I honestly think we'll get more participation.

So I would aim for working out the metrics first instead of trying a massive revamp in review process, and see if that will close the gap.

In theory I agree with you...

MGParisi's picture

But unfortunately the theory of this and the reality of this is two separate things. There have been many times I thought something was going to work, it had to work, but when it got out their it failed.

Im glad someone took action and tried to make this work, but right now we are standing on 2500 modules to be reviewed, and the only people who can approve them is Web Admins. You can mentor someone, but then after your done you have to find someone to promote that application from sandbox to website, and currently it is just starting to be realized how little of this process has been formed. In addition, as I know will be the case, you will mentor someone, and then ask for web admin approval and they will tell you that they dont feel comfortable because of X or Y, and then you will attempt to solve that, 3 more weeks pass and we all ran out of gas again.

Everyone will come in with their ideas on how things should work, and there will be a hundred issues and 10 solutions, and in the end no one will have the ability or willingness to say that we are doing it X way or Y way.

Its so exhausting most of the senior members of Drupal not only acknowledge that this is the case, but that just give up on any attempts to make things change. Including multiple people I know who was against this idea from the start yet is so tired of the process that they dont want to even partake in this conversation.

Chances are that will be the fate of this, or we can finally get some people to take some real leadership and make the decisions.

Maybe we should contact all 2500 people in that issue queue and get their opinions on how the review process is going. Unfortunately I fear that at this point most people would take that as poring salt on an open wound.

Leadership

I am not without a solution, we need leadership. We dont need a Democracy we need a republic. We appoint people to make the tough decisions and if they are available and continue to be available they can make the tough decisions or go back to their council and ask for a vote. Yes this will be a very active job, but at least we can cut out some of the endless chatter about how things can and should be improved and move actually get approval to go ahead, lets try it out, or lets not implement this.

That will at least solve One problem, maybe not the "application process" but maybe others.

Purposed changes

I would be willing to explore new ways to do the application process in the future, but right now I just dont want to see that many people coming onto IRC over and over again wondering "When will my web application be approved" I have talked to one guy around 4 times now. Accepting everyone is a horrible option, but its allot less horrible then having people wait for 3 months to get approved. So while we continue to debate this until it gets solved, lets at least get the people who have taken the initiative to fill out the application the ability to be approved.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

They would I think. Hm. That

Bastlynn's picture

They would I think.

Hm. That Web Admin point really needs to get cleared up. We need that backend working smoothly regardless of the actual review system approach we take, be it "quick-stamp and fix post release", or the current "get-it-perfect then release". I think we need to make module review and approval more open if we're going to leverage our membership. (See my note earlier about training new members to feel the need to review and approve at least one other person.)

Which means that I think we really need to focus the attention on clarifying that Web Admin process and see who we need to talk to to make that a less-bottlenecked process.

It sounds like you've been digging into it? What data do we have on the current process to become a Web Admin? What powers and obligations come with that position aside from the ability to promote a project/user?

For whatever help it is: Web

eliza411's picture

For whatever help it is: Web admin privileges are no longer required to approve full project applications - the role of Git administrator allows this as of last Friday (April 1, 2011). I don't know where that needs updating or who needs to be informed to make this information useful, though.

Awesome - thank you for the

Bastlynn's picture

Awesome - thank you for the extra info. Do you have any additional information about the Git administrator role or how one becomes one?

I'd suggest you talk to zzolo

eliza411's picture

I'd suggest you talk to zzolo ... he's likely to have the most information. Also, when you track this all down, documentation under http://drupal.org/node/636570 and/or http://drupal.org/node/21923 for how to get access to approve full project applications would prove helpful, I'm thinking.

Can we make a View showing

pwolanin's picture

Can we make a View showing all users who have that permission? i.e. all possible reviewers?

.

Michelle's picture

I don't know about all... I know there was me, ajk, killes, and kiam for the last few years. I'd assume anyone who's a full admin would have access as well.

There's a difference between who has the permission and all possible reviewers, though. That was the whole point of moving the review away from just ajk and I and into the issue queues. So now anyone can review. Still not very many that can act, though. I've pretty much entirely dropped out because I can't do code reviews and we never had any really clear guidelines to follow. I was always scared I'd approve someone just to find I missed a security issue. So I ended up just weeding out the junk once ajk started code reviews.

Michelle

Would you be interested in

Bastlynn's picture

Would you be interested in helping us with solving the bottleneck on reviews if we can develop a reasonable coherent proposal on how to? I'm thinking maybe you know who to bring a proposal to and what concerns we need to address to make it viable?

.

Michelle's picture

I've been saying since ajk started reviewing and the queue started bottlenecking that the review process didn't scale well. I pushed to get it opened up so we could get more reviewers and still it doesn't scale well. Personally, I'm in favor of a "golden contrib". When a project gets so many users, then have it peer reviewed and security reviewed and slap a badge on it. Beyond that, open the gates and slap on warnings. I've been saying that for years but haven't managed to convince very many. And, even if I did, who makes the decision? There's no one in charge, really. I guess if we can convince those who have the access to make the change they could do it, if they were willing to take the responsibility for making the change. Otherwise, it just goes around and around with some saying we should and some saying we shouldn't and the backlog continuing to build.

Michelle

Fair enough - it sounds like

Bastlynn's picture

Fair enough - it sounds like it's been a frustrating time chasing your own tail to get anything done on the matter. :-/

I'm willing to sit down to compile the problem and solution requirements, the proposed solutions, and outline a base solution based on how cleanly each given solution would apply to the requirements. That will at least get us down to something concrete to talk about. After that new solutions would have to clearly indicate how they are better (not just equal to, no lateral moves allowed) the front running solution in the proposal - hopefully that will cut down on bike shedding?

Then, once we've something solid in hand... uh - we'll get to go be obnoxious? ;) Hopefully not just me, I hope MGParisi will be along with, and others on this thread - but I really would like to fix this bottleneck. This isn't a responsibility that should be falling on a just handful of people, and given a reasonable course of action and requirements I'm willing to start hunting down the person who can put it in place.

Sound good?

Anyone can review. Our

Dave Reid's picture

Anyone can review. Our bottleneck is not people who can go through RTBC applications and enable the user's priviledges - it's reviewing.

Senior Drupal Developer for Lullabot | www.davereid.net | @davereid

I didn't know that. Thanks.

davidhernandez's picture

I didn't know that. Thanks. For reference: http://drupal.org/node/636570

Are you sure on that? I

Bastlynn's picture

Are you sure on that? I remember my review sat in waiting for a few weeks because the last reviewer didn't know who to contact to get it kicked over. Do we have an active list of who has the power to enable privileges? Has that been added to the documentation? (disclaimer: It's been a few since I last looked for that list.)

Mostly I'm interested in metrics - like what the current average wait is on a 'ready to approve' module is, and if that rate has been creeping up over time indicating that we need more people responding to that end demand. If the wait is long I'd like to know if it's because the user who can enable privileges is overwhelmed with the number of requests, or is taking additional steps that are eating into time. (Are they performing an additional review after the base user review - if so - why? If it's a matter of trusting the first review, what can we do to increase trust to avoid repeating work?)

I would love to be proved wrong here - it means I can mark off an entire section of things to try to address and we can cut right to working up ways to increase numbers of basic reviewers.

edit: to clarify why I was asking the question

Enabling User Priviledges

MGParisi's picture

I had an extremely tough time finding people who had privileges or that where willing to accept a project, or even explain the process, or knew of the process. One guy came on 4 times. Hell I even offered to get webmasters access so that I could do spam cleanup and other things so that others who knew the process could review code. Even if I had access I would be intimated to do anything because I could not get an answer on what to do.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

I with the basic suggestion

pwolanin's picture

I agree with the basic suggestion at the top, we should empower a lot more people to promote project application. If the review process is taking the reviewer more than 15 min, it's broken. Perhaps we can have a short, standardized checklist some place? Mine would look like:

  1. has a .module and .info file with reasonable content.
  2. module code doesn't contain license notices indicating it's just copied or might not be GPL.
  3. module is not trying to duplicate the functionality of 20 other well-known modules
  4. has some kind of README.txt
  5. module code is reasonably aligned with Drupal coding standards (by eye)
  6. There is no obviously unsafe code such as use of $_POST or $_REQUEST, or raw HTML forms.

I assume we are still in the (moderately unfortunate) bind that a user once approved can create unlimited additional full projects? Having the first few have to be individually approved would be better, as would a more aggressive process to re-assign or clean up abandoned projects.

Proper reviews

pcher1bw's picture

Code reviews won't kill Drupal, they may save it.

Dries' Keynote in Chicago clearly stated that we need to reduce critical errors as early as possible. Code reviews enable the reduction of critical EARLY in the development cycle, before the code is even integrated into the system.

Studies performed at the Software Engineering Institute at CMU indicate that fixing a problem found during the design phase will save 55 hours over fixing the same problem found during integration testing. Additional studies in industry (HP, Citibank, Motorola, IBM and others) support these findings. Peer reviews are one of the key process areas (KPA) that raise a CMM level 2 organization (projects are repeatable) to a CMM level 3 organization (Well defined processes, but not managed by statistics). CMM (Capability Maturity Model for Software) and CMMI (Integrated Capability Maturity Model, including systems engineering and human resources as well as software development) were predecessors to the Six Sigma quality efforts that engineering organizations attempted during the 1990's and early 2000's.

Proper reviews take time, especially if they are code reviews. Having come from an industry where quality was a key component, we reviewed 100% of all new code, and any patches that were larger than 20 lines of code. I haven't read the rules that are requiring this review, but where I previously worked design reviews were required as well as code reviews, coding could not start until the design review was completed. The rule in industry is that for each 100 lines of code reviewed, allow the reviewer 1 hour of review time. If a module contains 200 lines of code, expect a reviewer to need 2 hours to really inspect it well.

@pwolanin You need to add a 7th bullet : 7. There is no obvious or unnecessary performance hit in the code.

If you want to speed up the review process, I suggest that each module developer/submitter review 4 other modules from other module developers. A proper code review has at least 4 people reviewing the code, because one reviewer may not catch ALL the issues. Perhaps what could be developed is a tool to aide reviewers by listing each line of code with a line number and a button to add a comment. The reviewer can review the code in the tool click on the button and add comments where necessary. These tools already exist for C and C++ but cost money.

There should probably be a review issues queue for developers to sign up to review modules, or when a developer submits a module for review a list of other modules that need review can pop up and the developer can choose one.

Some suggestions for voluntary reviewing:
1) A senior person leads the review.
2) Coding Standards should be enforced, but coding style should not be mentioned unless it really makes the code unreadable (no flames, everyone is equal).
3) Have established criteria for security, performance and coding standards (OK, I know coding standards already exist).
4) Issues found should contain explanations of why they are issues.
5) Everyone involved should view the review as a learning experience as well as a quality measure (I certainly learned things in code reviews, even when I had been a software engineer for 10 years).

Paul Chernick

Paul Chernick
CEO
Chernick Consulting
(310) 569-2517

I think what you've got here

Bastlynn's picture

I think what you've got here is a great goal to aim for - but based on what I've seen mentioned on the thread so far, we're painfully short on people to implement it. We need more of them in order to get to the team code-review format you have in mind. So, call this a stage two or three solution while we work on stage one?

Any suggestions for steps to get the review population to this point?

What now?

fuzzy76's picture

I'm sorry to say, but your post might as well told a story of code-reviewing unicorns with kittens on top. In open source, you will never find volunteers enough for anything remotely resembling what you described. You are basically asking all developers on average investing two figured number of hours to code reviews. Of their free time, for no salary? I really don't get it.

Just like documentation

arianek's picture

Just like documentation right? Oh, no actually a growing number of people spend countless hours doing that (and did long before I got any subsidization for it). All it takes is doing some marketing to make this have more visibility, preferably with some more concrete structure on how to help. There will be people who enjoy doing code reviews (just like there are people who are involved in Drupal who enjoy writing and editing), who will help if they know what the process is, and know there is a need.

Hacking 101

MGParisi's picture

I fully agree with the concept/process you are proposing, but one of the comments to my early response in the “Module Reviews will kill Drupal” discussion was that I can’t expect volunteers (this is open source) put the proper amount of time into reviewing other peoples code. What hacker is going to follow THIS heavy weight process?

-pcher1bw
source: http://groups.drupal.org/node/141114

Now it takes allot for me to really get so tired of someone or something that I have to come in and club around. But then you see something like that and you just realized what you have been doing. Wasting your time. Now I know why the best chances of making good change is when no one is looking. Because if you have a really good idea, and you decide to come into D.O. or G.D.O. you are bound to run into people who are claiming that "Coding Standards should be enforced" when they have no idea what they are talking about.

BTW, for those that can not bother to use Google or Wikipedia, a HACKER is someone who does NOT follow traditional processes, coding standards, or well any standard. If they did they would not be HACKERS! And there is value THEY can bring too many projects.

So as a response to pcher1bw's post the "studies" he is referring to are in the context of PLANNED large scale business project's (Excluding RAD, Case Studies or Proof of concept). I would want to see the minimum amount of standards control at or around 50,000 lines of code (which is relatively small project). Also pcher1bw's case studies are referring to is large systems tailored to a specific situation, with a specific target audience. Also we must note that these companies understand their need to provide product distinction, and thus need custom software to do so. In no way are they talking about the tiny 10,000 lines of code that incorporates the average starting module. Please, again, make sure you know the context of what you read (and type). I have seen a hundred studies like the one Pcher1bw mentioned. Anyone who has gone through the basic MIS class will hear about them. Hell I was getting these examples when I was in Basic Business Accounting.

Normally when I see a post like Pcher1bw's I would assume it was a debating tactic to win an argument. Unfortunately the situation is much, much worse. He is making these statements without even understanding the situation.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

It ain't broke

greg.harvey's picture

I don't really see what the fuss is about.

Allowing sandboxing lets people create projects freely on Drupal's infrastructure - that's new and it's pretty useful. It at least allows people to move their development in to the Drupal fold without needing any special access. This I like, it's a positive change from the Git migration (not that it really affects me).

Apart from that, nothing else has changed ... right? People still need permission to create project nodes. You have to demonstrate you're responsible and competent before you get that permission.

This is a good thing!

Sure, it takes a while - it's a popular project and there is finite resource, so be patient. Once you have permission, your modules are not subject to review - you can do what you like, as long as you don't start making a nuisance of yourself. But that initial review is really important.

I went through this process and I am here. webchick, merlinofchaos, chx, jeffeaton, etc. etc. - almost anyone you'd care to name (unless they were in really early) have gone through this process. It took me ages to first get CVS access, but it didn't stop me waiting it out. In fact, I'd love to know how you seemed to duck around it, having joined later than I did. Looks like you just dropped lucky, got reviewed by someone who wasn't too bothered about your code quality (surprisingly, since you say it was chx) and happened to apply during a quiet period.

Anyway, I don't see how the current process is broken. You can't just throw projects open because it would be a total mess for a whole host of reasons! I really don't see a problem here.

The problem isn't the

Bastlynn's picture

The problem isn't the reviewing itself. Vetting work before allowing access is a good control and I don't think anyone has argued it isn't - in a perfect world.

The problem is the implementation we currently have. We just aren't getting through applications fast enough. It's not keeping up with the crowds.

Which means:

1) We need more base users to review applications promptly before we lose the new coders because they didn't get any response.

2) We need more empowered reviewers who can take a project reviewed by a basic user and finish the application process to grant powers to the new user.

3) We need an actual process to create more empowered reviewers from the basic users who have demonstrated interest and competency in reviewing, preferably in as automated a way as possible to avoid further bottlenecks.

4) We need a way to review who has the power to grant an approval, and a way to make sure they're using that power and haven't gone inactive. (So we don't have a miscount of how many empowered reviewers we have.)

Once we have the system in place - then we can start work on making reviewing a more common activity around the site.

Hmmm

greg.harvey's picture

The problem isn't the reviewing itself. Vetting work before allowing access is a good control and I don't think anyone has argued it isn't - in a perfect world.

The opening post sounded to me like it was arguing exactly that. But anyway...

The problem is the implementation we currently have. We just aren't getting through applications fast enough. It's not keeping up with the crowds.

I'll buy that, the project is getting bigger after all.

So, with that in mind:

1) Absolutely - what we could have would be small army of people who only check if a module duplicates something already on Drupal.org - if something similar exists, direct the person to open dialogue to becoming a co-maintainer of the module and working with what's there already, supply feature requests and patches. Otherwise it goes in to a smaller queue for serious review.

Everything else we can worry about if (1) doesn't do enough - fix things you can quantify are broken, don't imagine what might be broken and start fixing it before you know if it's even a problem. That's classic developer behaviour... we always want to create things that no one will use and fix things that aren't broken. It's in our nature. ;-)

I think the original post

Bastlynn's picture

I think the original post identified both problem and proposed a solution. Which is awesome, but can make it unclear where a question is being directed. ;)

So - have any suggestions on how to get more people aware of reviewing, to get them aware of how to review overall, how to identify those who really pull the load on reviewing, and how to reward reviewers for their efforts so reviewing becomes a recognized contribution?

Keep it simple

greg.harvey's picture

Ok, you've asked for my suggestion - here it is.

I don't think point scoring and rewards are necessary. They're not anywhere else in the project, so I don't see why reviewers would need this any more than, say, docs writers. I'd just implement something that requires no effort from anyone to realise - just a subtle formalising of process. How does this sound?

  • anyone can keep an eye on the Project Applications queue: http://drupal.org/project/issues/projectapplications
  • official "reviewers" don't look at anything until it's marked 'needs review'
  • if someone marks their own issue as 'needs review' it should be bounced straight back to active with a note saying "you can't pass yourself!"
  • any other user can either mark it as 'needs review' (e.g. this is sufficiently unique that drupal.org would benefit from having it) with their reasoning
  • only then does it get a proper review
  • if it is a clear duplicate or utter c*@p, it should be marked 'closed - won't fix' with an explanatory note (what module it is too similar too, how to help that module instead, some other reason) - you don't need a full code review to achieve that

That way the whole community can sort out the wheat and the review team can focus on it.

This way you don't change anything, you just formalise a way of working using the tools are already there. Write that up nicely, give it a catchy tag and start talking about it to anyone who'll listen, at Drupal Camps, DrupalCons, on Twitter, Groups, etc. etc. and it'll fly or fail. If it works, people will run with it - if it doesn't, it'll die and it doesn't matter.

Obviously that's a rough and hurried idea, but that's the sort of thing I would personally recommend to start to try to dent the queue without needing to make any big changes to the way people work.

Thank you for the clear run

Bastlynn's picture

Thank you for the clear run down on your solution to this. :)

Based on this and some of the other directions the conversation has taken on this thread, I'm starting to agree with you. I still think having a list of 'people I've helped mentor' would be really cool from a social point of view, but it might not be as necessary as I thought initially.

edited: for clear speech - it's late here ;)

Sure

greg.harvey's picture

It's one of many possible solutions, but the fundamental point is this - I believe nothing will happen if significant work is required on anyone's part to realise this, because no one has the time.

So whatever the solution is, it needs to be really simple at first and use the building blocks available - those building blocks are:

  • Drupal.org as it is today
  • the community

Any solution that requires anything more than that is not going to happen. Sure, complexity can be layered on once a simple model is working. But new content types on Drupal.org, adding issue categories, re-structuring docs, new menu items, new user roles, all these things take time to implement and test that the infrastructure guys simply don't have.

In the future you might ask for another tickbox on user profiles for "I mentor people", and various other things that might emerge as 'nice to haves' but are not fundamental to solving the problem. We need to work with what we've got to prove a process works before we start asking people to make tweaks to d.o to accommodate those processes.

Actually - a big thing that

pwolanin's picture

Actually - a big thing that changed is that a sandbox can be promoted to a full project. If it hasn't been made clear yet, everyone who is applying should be creating a sandbox project with the code they are using as the basis of their application.

So, ideally the sandboxs will make the initial review easier since one can have a dialog (including issues, commits, etc) in the context of the sandbox project.

However, as above, I think the review should typically be a matter of minutes - if not, then the code stinks and is not ready for general use.

Not that long

Michelle's picture

According to my IRC logs, ajk started doing the code reviews in early 2009. (I don't have my really early logs handy, but I have a couple from June 2009 where we were clearly in the early stages of doing reviewing) Before that, we just checked that the project was sane, functional, didn't duplicate another, and wasn't an application to work at CVS pharmacy. :)

So this whole process that people look at as "the way we do things" actually has only been done that way for a couple of years because one person decided it needed to be done and started doing it. Before that, getting a CVS app was pretty quick. I don't remember exactly but I'd be surprised if we were ever more than a week behind. Of course, that was before the floodgates opened. But that's the problem, really. Two things happened at the same time: 1) We started trying to do detailed reviews of the code in every application and 2) Applications started growing exponentially. When you add 1 + 2 you get months long waits for approval.

Yes, sandboxes have helped. Instead of having to stick their code in an issue, they get version control and and issue queue. And that's wonderful. But then they sit for months wondering when they will get to be a "real project". Maybe what's needed is for some better press for sandboxes? For having a project be sandboxed not have the same stigma it has now? I know a lot of people will yell, "No no no" to this but maybe what sandboxes need is real releases. They've been made almost into real projects but not quite and they've got Pinocchio syndrome. Why not take that one next step and let them have releases? And, yes, I realize what I'm suggesting here is a de facto "golden contrib". And I don't think that's a bad thing.

Michelle

I actually can't find my

greg.harvey's picture

I actually can't find my application issue, perhaps it was by email? Seems it was some time in 2008. I remember it took quite a while. That said, I don't remember exactly what was involved any more, but I'm fairly sure the code of this module was emailed for review: http://drupal.org/project/filefield_upload_limit

Hence my impression nothing much has changed!

I guess different people were applying different rules back then, because what you describe was not my experience.

Took me all but 15 minutes.

MGParisi's picture

Took me all but 15 minutes. It actually took me quite abit longer to learn Tortoise (back then it was CVS), and I think I was given access to something I wasn't suppose to or a level of access, and I accidentally made a mistake that got a few people very angry with me:(


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Looks like I was a little off

Michelle's picture

I must have been mis-interpreting that little bit of log I found. I went back to my old computer and dug out my older logs and he started doing code reviews in October of 2008. So a bit earlier than I thought but still not that long of a time. Also in that log we were talking about how people were complaining about the lag and that AJK was going to add a note to the instructions that it may take up to 3 days to be approved to try and help with the impatience. :)

In my going back in history, I also found a very interesting email from Dries back in August 2007 talking about how we need to remove bottlenecks to contribution. That was in reference to dupes, not approval time, since that wasn't an issue back then, but the principle is the same. Since it was a private email, I don't want to quote it here without his permission. But it does make me wonder if he's aware of just how bad it's gotten and what his thoughts on it are...

Michelle

I think mine was probably May

greg.harvey's picture

I think mine was probably May 2008, but sounds like it was 2008 people started to think about doing code reviews, even if it wasn't official, before handing out CVS accounts - because I got one. Actually, I didn't mind at all. It was quite nice to have someone look over my code and give me some pointers. =)

I would accept Michelle's solution

MGParisi's picture

I would accept Michelle's solution. I think its reasonable. Not perfect, but if compromise is needed, then this is a good compromise for me.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

I like using the sandbox like

Bastlynn's picture

I like using the sandbox like that as well as a long term answer.

In no small part because the approval isn't dependent on usage count - something more specialized and/or less advertised modules might face as a barrier.

The other benefit here is that part of this solution can be implemented immediately with good documentation and clear guidance for reviewers without too much extra effort. Really at this point based on the various paths this thread has taken, I'm wondering if what we really need is just a good rush of documentation and a project/activism to raise awareness and encourage action.

Does that match anyone else's take on things?

Compromise?

MGParisi's picture

I dont find the current sandbox system to be adequate. But I do find the changes that Michelle suggested to be a reasonable compromise. Its definitely not everything I want, but it would at least be a step in the right direction for current developers.

However until improvements can be made to the sandbox, I would want to see the system opened up.

I think there is allot to be learned from this current approval system. Some is good some bad. But an approval system that can not be maintained by the volunteers who work on it is not a solution for success, its a recipe for disaster.

There is allot more to this problem then a few patches to the sandbox will solve. But I am willing to accept this small compromise. Unfortunately we do not get a full picture from this post. Many of the people who comment had the patience to deal with the current system or came before the current system.

We have certainly lost people to other projects and therefor we will not hear their cries. Essentially our current system has already effecting the sample of the people responding to this post.

*edit, changes to improve accuracy!


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

The problem is once we allow

Dave Reid's picture

The problem is once we allow them to create releases the Security Team is now responsible for any 'official' releases.

Senior Drupal Developer for Lullabot | www.davereid.net | @davereid

.

Michelle's picture

No, because they aren't official releases. They're sandbox releases. The idea is that sandboxes will have all of the same functionality as regular projects but not have been subject to any sort of review. When a person goes to search for the module, they can have the option of searching sandboxes as well with the warning that anything found there has not been looked at by the security team nor gone thru peer review.

With sandboxes fully functional, the maintainer can choose whether they want to go through the peer review process to become a full project or not. As it is now, they don't have a choice if they want packaged releases. And, yes, I have heard from multiple people that they can't tell people wanting to use their module to get it from git. They want people to be able to download a tar/zip.

If we make sandboxes fully functional, then the difference is whether it has been peer reviewed / falls under the security team. And it makes the wait more understandable if the whole point of what they are doing is to get reviewed not to get a usable project.

Michelle

See below

greg.harvey's picture

Michael Cole's idea below would help with this problem. If you said only 'reviewed' releases are the responsibility of the Security Team, and anyone could release but only 'reviewed' releases were recommended, that would maybe work?

Edit: which is pretty much what Michelle just said while I was typing this... ^^ ;-)

I disagree with the comment "It ain't broke"

Mikey Bunny's picture

I've been waiting over 5 1/2 months. I fixed up the items for the last reviewer within days of them looking at it and have not heard anything for over 5 weeks. I'd say that was pretty broken.

While I've been waiting I've been reviewing two other modules and I'm sure there will be more before mine gets looked at again.

2,500

greggles's picture

Where did you get the 2,500 in this line?

We have almost 2,500 modules to be reviewed.

Thanks.

Nope

MGParisi's picture

Right NOW, it is at 217, it was almost over 5 pages (50 per page x 6 pages). But yes I made a mistake in writing this. In fact I even replied (as you can see above) and typed out 50 x 6 = 3,000. Unfortunately no one saw that and me until now (18 hours later), and this now beign the hot topic of the week, how many people have viewed this page, yet could not find this mistake?

I think this is an example of why a coding review process is unreasonable, unless you expect the same amount of attention by the same number of users this post has gotten for each application.

This was an honest mistake, and unfortunately I think it will do allot of harm to the entire issue. I have since corrected both errors (the 50 x 6 = 3,000 and the 2,500 number).


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Poopy

MGParisi's picture

Unfortunately when your asking for volunteers you dont put them on wait for 3 months. Now the topic will be derailed about My mistake, my infinite number of motives behind my mistake, and/or the fact that putting 250 people on wait is not a big deal. I do apologize, if this figure does change people stance on the topic then that is a very big deal. Ive re-read that post a few dozen times, and even ask for advice to prevent a mistake like this:(


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Wrong again

pcher1bw's picture

I'm sure several people spotted it and were just too polite to point it out, just like your problem for using their instead of there and a few other problems you have with English. It would be very interesting to look at your code.

Paul Chernick
CEO
Chernick Consulting
(310) 569-2517

Anal Retentive

MGParisi's picture

Who are WE to judge what someone else finds acceptable or event excellent. Who are WE to decide what is a worthy project and what is not necessary in the world. Who are WE to put up boundaries on innovation and progress.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

.

Michelle's picture

Can we keep this on track, please? Coding standards are important. Perfect spelling and grammar in discussions is not.

Michelle

Sorry

MGParisi's picture

I was going to change the title of that post because I was going to say that I am anal retentive. But your reply makes editing my title impossible.

Are coding standards now required? Personally I like tabs not spaces, and I do if statements differently then just about everyone else too.

example

if (X)
{
X=X+1
}

I also write very universal SQL that would run on all four popular databases and Foxpro (access is the only db that requires a few cases of if statements). I learned My style of SQL through compatibility work between MSSQL and Oracle. I am also very standards driven, and will only write OOP code, but get frustrated by PHP's limited OOP's ability's.

So I am ANAL RETENTIVE (which was the title of the post two up, and was going to be about how I am Anal Retentive). I do apologize, I did not mean to have inferred someone else was anal retentive.

Given all of this, I find it strange that I am the one advocating for taking the stance that standards can limit individuals and that hacking is as equally beneficial then my highly intensive standards processes.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

.

Michelle's picture

They aren't a requirement in contrib, but it's part of best practices and strongly encouraged. When getting your app reviewed, though, not following coding standards will make it a heck of a lot harder if not impossible. I haven't been following the reviews, lately, so I don't know if they're approving apps that don't follow them or not. But it's more difficult to review someone's code if they don't follow standards which means a lot of people may not bother and just move on to someone who does

Michelle

BTW

MGParisi's picture

I also do have a disability in written communication, ADD and Aspergers. I do attempt to give people the benefit of doubt and thus attempt to reply in a non offensive way to your response, but I think you should be aware that some of us are a bit eccentric.

I could of made excuses for my error but instead I chose instead to apologize. I definitely do not want to harm anyone, and I try my very best not to.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Good to know

arianek's picture

Hi MGParisi -

I just wanted to acknowledge this, and say thanks for outing yourself and your condition. I think this is actually really useful to know when communicating, so we can help to read this with the appropriate tone. Just like my health conditions prevent me from accomplishing some things (and I like people to know that so they don't make assumptions), it's good to know that someone's communication-related challenges might affect their responses as well.

No excuse for actual insults, etc. (which you've clarified you weren't doing), but like I say, good to know. ;) Not to derail the thread here, but if you have any advice on how to communicate effectively with you and others with these sorts of conditions (I'm sure you're not the only person in the community with these), I think it would be fantastic to hear more about this, maybe here: http://groups.drupal.org/diversity-outreach-recruitment-project (I think this might be an appropriate place to post, as it's meant for increasing inclusivity.)

Done

MGParisi's picture

Ive been asked a half dozen times to contribute to that project, I fear its repercussions. For what its worth here it is http://groups.drupal.org/node/140069


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Thank you and Sorry

pcher1bw's picture

@MGParis

Thank you for your story at http://groups.drupal.org/node/140069. If I had known, I would not have pointed out the problem with "their" versus "there".

Paul Chernick

Paul Chernick
CEO
Chernick Consulting
(310) 569-2517

Hi, why are we creating this

MichaelCole's picture

Hi, why are we creating this pressure on ourselves?

Collaboration starts with "yes", not "no".

Want to write a module? "Yes" go ahead and write them, and they all get project pages - that say "un-reviewed".

Want to review a module? "Yes", go ahead and do code reviews on them. After review, they get a "reviewed" badge on the project page.

Want to use a module? "Yes", you can search for "reviewed" or "un-reviewed" modules (reviewed by default).

This would be a non-issue if we were measuring module quality on a scale with more than two values: "yes", and "oblivion".

Mike

+1

fuzzy76's picture

I agree, this sounds like a great idea. This would also mean that once someone got their first module approved, they would still need to get their next module approved.

I agree

lloydpearsoniv's picture

+1

+1

MGParisi's picture

I like how you worded it.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

After reading this thread, i

meba's picture

After reading this thread, i decided to comment as well. One thing that struck my mind was greg.harveys comment about the current process: it has been there for a while and we still have the chx's and others.

Second thing - i tried spending some time in the cvs queue couple of months ago and I wasn't able to deal with the amount of crap people submitted for review. That said, I had trouble helping them as I wasn'tsure what the proper process is.

Third - I like the idea of just spending time promoting the work after the docs are created. Some did that very succesfully before (I am looking at you, documentation team) and if somebody tweeted once in two weeks 'hey, fancy a module review'? I am sure I would review one every time.

Last - I love the 'i scratch your back you scratch my back' idea. What if linking to your review of another applicant's module is part of the application guidelines? That's extremely easy to implement and can happen in days.

/me hifives on the Docs Team

arianek's picture

/me hifives on the Docs Team nod. ;)

Seriously though - I was saying this on IRC the other night while a bunch of us were discussing this topic. Starting the @drupaldocs Twitter account (in addition to Jennifer and I doing quarterly frontpage posts and having more visibility at events) has been the single most effective tools for getting Docs Team growing again.

I use it for three main purposes:

1) Advertising Docs-related events (sprints/sessions at cons and camps), news, and special posts (like the front-page updates, photos from sprints, etc.).
2) Posting occasional issues that need attention, usually ones that are more technical than my abilities or are at "needs review" stage. (...that reminds me, I should ramp up the issue tweeting again!)
3) Retweeting any tweets that people @reply to the account.

It takes hardly any work once it's set up and you follow a bunch of people initially, and it's very effective. Bet it would help a lot for getting some visibility for code reviews too if someone was willing/able to take that on.

[Update: just looked and I started the account July 15, 2010, and have just surpassed 500 followers. Not bad!]

I know I became more

davidhernandez's picture

I know I became more motivated to help with Docs and other things after DrupalCon, because I met/talked with people in person. Were there any module review sessions or discussions at DrupalCon? I definitely think advertising is important. As I posted earlier, I didn't even know everyone could help review modules. I assumed it to be something requiring a higher privilege.

As far as I know, there was

arianek's picture

As far as I know, there was just the small code-review group at the back of the Docs sprint room in Chicago. The challenge there is that I can count on one hand the number of people who had been regularly doing code reviews for CVS applications prior to the git migration - I only know of one (zzolo) who was at the conference, and he was the one to lead that mini-sprint.

It's a huge issue of lack of wo/man power, which also explains why there's not a lot of people who understand what the process is! I bet if you (or anyone else) wanted to help with documenting the process and advertising it a bit (on g.d.o, perhaps blog posts on the Drupal Planet, forums, twitter, etc. that would be immensely helpful!

zzolo is extra busy right now doing Code for America, so I would imagine any initiative would be super well accepted!

Done. I'm in. While the

Bastlynn's picture

Done.

I'm in.

While the debate continues on what shape in the long term the review process should take, I'd like to get started on immediate relief using the current system. If the system changes later as a result of discussions here, that's fine - but that doesn't prevent us from taking a grip on the current system and doing what we can with it.

In that light:

@arianek Do you mind if I ask you for advice on the process of kicking this forward into an Initiative with some weight behind it? This'll be my first here on d.o so I'm not sure where my first steps ought to be to take this from just a thread into a properly set up work project. (I get the 'just do it' mentality of drupal - I'd just rather not trip over my own feet in the rush.)

I'll be contacting zzolo with questions and a proposal on action tomorrow morning.

And begin review of current documentation in hopes of getting something more coherent as guidance for reviewers under the current system by CoB Friday so people can look it over.

Documents verse Modules

MGParisi's picture

Arienek, I appreciate your insight, and welcome your ideas on promoting a volunteer mentoring project... I have read this thread a dozen times, and it just came to Me.

There is a LARGE hole in the logic here. Documentors do not need to wait 3 months and go through a lengthy review to edit a document. What do you think would happen to the state of documentation if we required a 3 month mentoring process before edits could be made?


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Hiya - YES. You are totally

arianek's picture

Hiya -

YES. You are totally right on that. But get this:

In the gap where there wasn't really an active Docs Lead (before Jennifer and I were officially appointed), there were TONS of docs issues that had been stagnating for many, many months. Many of these were applications for Docs Admin role. Because of this, docs were very behind, and this is why when we launched D7 we still hadn't caught up on the docs side. One of the first things I did once I had the appropriate permissions was to approve a whole bunch of docs admins, and then set the immediate priorities (for the core D7 docs). Then getting the actual help and volunteers rallied has taken an ongoing and constant effort.

We are STILL playing catch up on the docs queue after many many months (or even a year now?) of being the main docs queue maintainers.

Why did this happen? Because the previous lead was ready to step down but there wasn't really anyone to take on the role until Jennifer and I stepped up. We talked to the outgoing lead and with her blessing, basically nominated ourselves to share the role (which was later approved by Dries, who formally appointed us). Docs suffered exactly the same fate as the code reviews, because there wasn't anyone dedicated explicitly to this task who was driving the work forward, but since we've taken this leadership role, things have been rolling along quite well again - now with a large group of enthusiastic and skilled Docs Team members (who are becoming ever more self-sufficient with our support).

It really is a do-ocracy in Drupal, but it is not free of hierarchy - anyone can argue that this being open-source, it should be an organic self-organized group of people. But the reality is that things don't get done that way! People need a bit of support from others with experience, just to give the occasional advice or confirm they are on the right track. Especially in areas like docs or code reviews, where there can be less clarity around how to help. (We've documented a lot of this now for docs so there are resources for those who go looking.)

Things get done when a few people who care a lot about something decide to take responsibility, and do the work to get momentum and a process in place that will actually work.

If any of you are thinking of helping with this, my advice is this: Keep it simple. Start off with the essentials and a small group of dedicated people, and don't spend too much time hashing out details. Get consensus on the basics, document them for the wider community to see (for transparency's sake) then get going and start to get the work done.

Oligarchy

MGParisi's picture

As I said, Drupal is more of a chaotic Obligarchy combined with a Do-ocracy. Yes we have a leader, and some leaders for some tasks, but most things never go anywhere because there is either no one paying attention, too much content, or no one to contact to get approval. Essentially we don't know who our representatives are nor do we know the rules to play by. Its a very chaotic thing for Me. I wish we had more of a republic. Even if its a Dictatorial Do-ocracy Republic.

I was in the same issue a few years earlier with the issue queue, and probably stripped the issue queue down to 3-4 pages. Most of the remaining tasks where admin level tasks (which I could not do), and so things slowed. Those issues probably sat there until you came along. I hope you are with Drupal a long time, I wish I could dedicate myself to one cause, but I dont have that ability. I jump into a job, get what I wanted done, then move onto the next job.

But realizing that if we had a peer review committee for Documents and what that would do to the ability for the site to exist, I think is a powerful example of what I see about this issue.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

In your last paragraph "...if

arianek's picture

In your last paragraph "...if we had a peer review committee for Documents and what that would do..." did you mean for code reviews? (There are methods of peer reviewing docs, but I won't go into explaining right now in case that was a mistake!)

I too hope that I work on Docs for a long time! But I believe that even now, there are other Docs Team members who are getting confident and knowledgeable that we could have a smooth handoff of responsibilities if it was needed. There is a much better structure and flow now than a year ago.

Jumping around isn't bad either, I think it's helpful to have someone jump in and knock through a bunch of issues if they can!

I do think of our Do-ocracy as having a bunch of benevolent dictators ;) Dries and those appointed to leadership positions are just that! I think that anyone who puts in enough effort and a good argument for why they should be appointed some decision-making power will be taken seriously.

Continual Issue

MGParisi's picture

Sandbox Verse Released -
Regardless of what you want it to be, right NOW the only distinction is Sandbox or Released. The distinction between the two is if the people who have gone through the review process and thoose who have not. There is no standard of people who have access to create a Released module. Some (like me) got it in 15 minutes with pretty much no guidance. Others took months, and went through a rigorous process.

Mentoring -
If the process of reviewing is one of Mentoring, then what is this process. We have no requirement of following standards, its simply "Highly Recommended", so what are we going to "Mentor" people in?

Quality -
If the process of reviewing is a focus on quality, then isn't it a module by module basis and not a one time review process? What if an update comes out, do we then have to re-review it?

Standards -
There are no standards for the review process.

Accepted or Oblivion
As stated so eloquently, we have two options. It is either Accepted, or the entire project goes into Oblivion.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Quality

MGParisi's picture

As you learn about the entire system, and map it out (which I can do in my head), that the only solution is to actually remove the entire review process. Any possible process only serves to limit access to release based on a non-existent, non-consistent objectively human based process (Black Box). This seems silly. Next the notion that Sandbox items dont deserve security team attention also seems silly. Why are we putting resources into this impossible goal of reviewing modules and not into security and patching?

There are no possible systems that can be based on the current review system. Don't take my word for it, I challenge someone to come up with a working system. I prefer Data Flow Diagrams, but you can choose your tool. Ultimately you find a lot of contradictions or extremely process intensive tasks, and the over all result is spent resources with little to no gain in quality improvement.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

What is Quality, Value, etc.

MGParisi's picture

This will be VERY simple... I studied this for 4 years in collage and could easily spend the rest of my life in Academia learning about Quality. It is also not a science or a factual topic. I can not sit here and say this is the answer. If you wish too debate this, we will be here all week... this is just to get people to start to think about the topic I hope it works. Please refrain about topics of quality here unless they pertain specifically to the topic, start a new thread if you would like to expand this idea.

Value

What we are triing to achieve is Quality. But I think we need to discuss what quality is. I think its best to start with Value. Value = Benifits - Cost

In some cases this is clear, Value = Savings of Coupons - (value of time * time it takes to cut and organize coupons).

But in most cases it is not so simple. We then must enter into the complex realm of Quality.

Quality

There simply is no one way to quantify quality. Quality is objective not scientific, its heavily based on perceived, expected and actual results. These can be influenced by marketing, productivity, communication, charisma, customer service etc. Standards are not quality. For instance, Rapid Application Development offers allot of Quality for certain situations, while a highly intensive ISO, S3, or other process intensive development strategy provides a different layer of quality.

There are also trade offs. Often times Flexibility comes at the cost of processing requirements. Highly customized, highly tuned systems that meet very specific goals achieve great performance but offer little flexibility. This is known as the art of Computer Programming. Its where computer science stops becoming a science and more of a balancing act.

Examples:
- I think this illustrates it perfectly. A Discussion of Quality http://groups.drupal.org/node/13629 aka "Wake up community - Wordpress.org should scare you!"
- Box wine is of high value to some (low cost verse taste), while others would wont drink it.
- Mc Donolds Verse Wendi's. I hate Mc Donalds, they offer low cost food at that I believe is barely edible, I dont mind spending a bit more on Wendi's higher quality food.
- Some like to shop at Walmart. They dont mind the lack of customer service because of the low cost. I prefer Wegmans, which specializes in customer service but costs more per item. I simply hate standing in lines waiting to check out.
- Discount Cloths Verse Name brand cloths. I honestly don't see why people shop at Ambercrombie and Finch. I find it ridiculous to spend that much money on cloths I can find much cheaper elsewhere.

Processes/Systems

A Process (or System) is an interaction of people sharing information and performing tasks. Quality processes (Like ISO, S3 and others) recognize that their is no one size fits all. They are highly thought out processes that allow for a great deal of flexibility in appropriate area's to achieve "Quality".

Make no mistake, we are having a discussion about developing a system to create a level of quality. My stance is firm, Drupal.org can not dictate what type of quality process We use in other peoples extensions (modules and themes). Why? Everyone programs differently. Results, Not Standards or Methodology matters. Security is paramount. Flexibility verse Speed is debatable. To put it simply there is more then one way to skin a cat and everyone programs differently. We are very individualistic in our programming. Some demand OOP, others hate it. I like rigid types, others like loose types.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Idea - Documentation

MGParisi's picture

I purpose creating a documented process to improve quality and to understand the system we create to resolve this. The process was not documented and without a documented process, reviewing the success of a project like this and improving it can not be accomplished. Just understanding the permission system needed to grant people the ability to move a product from sandbox -> Production has been a quest onto itself.

What we need to do is to:
Document the Security Requirements necessary to grant GIT access.
Document current process and setup success parameters.

Once we have that then maybe we can break this stalemate of ideas?

Note System - http://en.wikipedia.org/wiki/System Please do not equate the word "System" to "Computer System". A system incorporates the sharing of information, actions, procedures, policy and automation which can include computer logic or human decision making.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Given the above...

MGParisi's picture

Why are we asking our coders to review modules that may not be used, instead of improving modules that are used? Why are we using resources on non-volunteer based review process? Do you actually think forcing unwilling participants into a process makes things better? Wouldn't our resources be better used helping those that ask for help rather then those who reject help?

We are NOT a democracy, we are a do-ocracy. This is not My Opinion This is the way it was, is and will be. This is NOT up-to debate, if you do not like it, you might as well fork Drupal, because the core individuals of Drupal will not negotiating on this. No amount of voting, debate or arguing will change this because like I said, its not a democracy. If anything Drupal.org is more of an Oligarchy then a Democracy. Dries owns the trademark thus controls the core, website's, domain and everything else to do with the term "Drupal" and the domains "Drupal.com" and "Drupal.org". Again, if you do not like this, then your only option is to Fork Drupal (as allowed and intended by GNU/GPL).

The only way any of this will change is by direct action from Dries.

Given the discussion on mapping out the processes (taking into account all of the factual data discussed) and the impossibilities it presents, the goal of achieving higher quality of services and of modules, and the fundamental fact that Drupal is a Do-ocracy the situation is narrowed down to 1 option. Open up the modules. After going through all possible presented ideas, I dont see any other workable solution. If you want to prove me wrong, provide a solution and diagram the process. Tell me how it will work, and what quality it will provide. Offer real world examples. I will be more then willing to review alternative options.

There are REALISTIC tools that we can utilize to ACTUALLY improve quality of service and improve information quality while removing the current systems illusion of quality. I am not talking about theory, I am talking about tried and true methodology (Spend the next 3 months researching Open Source and CMS systems, and you will find that these things work).

Improvements


"Review" to "Mentoring"

Now we should look at prevention rather the reactionary strategies. For that I love the idea about create a "Mentoring" program. But the mentoring program is a volunteer program, not a required program. Again I refer to the discussion and topics above.

Information

We have the ability to improve standards and to provide information about quality of the module. First we can see how actively maintained a module is. Is there open tickets, are the tickets being looked at by the module owner, is he supporting it, is their adequate documentation. Some of this can be provided via graphs and other data collection routines. To help provide a clearer picture of the module, its abilities, safety, etc.

Feedback and Reviews

We have thousands of users who are using these modules. Who better to judge a module and provide feedback on a module then the users. Allow people to provide feedback and opinions about modules. Also allow for voting (like a 5 star voting module), but instead of showing an overall average score, show a month by month (or Quarterly) graph of how good the module is. That way as new revisions are released they are not restricted by out of date votes.

Promote Mentoring

If someone gets mentored, they get a status on the modules they maintain and their profile that states they where mentored and by who. Mentors also get recognition for mentoring individuals, thus providing a duel incentive to be pro-active instead of re-active.

Search

Utilize these features into search. Allow consumers of modules to decide if the module is good enough.

These things work. They worked great on Drupalmodules.com! The failure of Drupalmodules.com was to not link feedback with version, and to not link voting with time and/or version. We now goto that site and see rankings that date back to D5. That is not helpful. We can do better. The ranking system needs to be time/version sensitive.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Mentoring vs. Reviews Ben

arianek's picture

Mentoring vs. Reviews

Ben (mlncn) and I think zzolo and I had talked about this ages ago (maybe it was even at the Core Dev Summit in CPH when we talked about the changing review process?). I think it's a really great idea to make the reviews more basic and against a strict set of criteria, and then to encourage optional peer reviews/mentoring that are much more in-depth.

Then the bottleneck is lessened, but people who want more support have that option too.

Review parameters

laura s's picture

the only solution is to actually remove the entire review process

This is something I've been pondering. Here are the challenges we face now:

  • Reviewing take time and energy. A cursory glance may not reveal a very bad practice buried in one line of code.
  • Having applicants review each other strikes me as having potentially the blind leading the blind. It seems that at the very least reviewers should be already vetted to have authority. Otherwise project approvers have to review the review to see if it's really a decent review.
  • There are things we don't want to host on drupal.org: malicious projects, projects that violate GPL or third-party trademarks (both of which raise legal hassles the Drupal Association, who's left holding the bag, doesn't currently have resources allocated to handle beyond on a general policy level and ad hoc needs)…
  • It's not clear that it's feasible, or even desirable, to implement any serious security review on projects that may have several hundred or even thousands of lines of code. Even vetted projects aren't subject to any security review unless they have a stable release, and even then it's not like the security team has resources to officially vet all of those.
  • Poor code quality in general is also something of a thicket in a pre-approval process. Do we really want to get into that with just a few people?
  • That leaves spaces-vs-indents.
  • Some people get worked up over perceived duplication. On the other hand, duplication could also lead to innovation. It can be argued that it's better to let the allegedly duplicate project move forward and the community at large can work to help projects merge resources, rather than what basically amounts to a "first dibs" kind of situation where, if someone else worked on the idea first, you have to work with the "king of the sandbox". That does not encourage contribution, imho.

But are we ready to just let the floodgates open? This process was originally just supposed to help make sure new developers/themers have the basic chops and understand the basic best practices before they create a dozen projects. This really is not a project approval process but a developer approval process, because once the developer is vetted, he/she can create all kinds of projects (which is a good thing imho).

So where's the balance here between resources we can feasibly allocate as a volunteer community vs. a process that helps enable and empower the kinds of contributions that benefit the community?

Laura Scott
PINGV | Strategy • Design • Drupal Development

Hopefully at some point we

arianek's picture

Hopefully at some point we can rely on automated tests for some of those more grunt-work like reviewing tasks!

Just wanted to make a comment about peer code reviews and "blind leading the blind" - we should not assume that everyone participating in that would be inexperienced. There are more experienced/long-time community members who enjoy reviewing code, and I believe if there was a clearer system for engagement (like Leisa said below), that they would help with this task. It would be framed more like developer mentoring in this case, and I think would be a very useful and meaningful contribution.

That was my point, and I

laura s's picture

That was my point, and I apologize if that was not clear. "Blind leading the blind" is only a potential situation if we let anyone review anyone (which is a policy that also allows for gaming the system: "I'll say yours is good if you say mine is good, and we'll both get approved").

Laura Scott
PINGV | Strategy • Design • Drupal Development

Ah, I didn't catch that -

arianek's picture

Ah, I didn't catch that - you're totally right, as much as I want to give everyone the benefit of the doubt, we really must have some system in place to make sure that doesn't happen!

Natural Reviewer

MGParisi's picture

We do have a way to review projects for Malicious code, or code that does not meet requirements (Like GNU/GPL). The public who uses the modules are most likely to find problems within the project. Most people use the template system, hooks or other features must have at least a basic understanding of the code. Therefor the public is most likely to find any security issue or other problem.

Copyright infringement is up to the contributor. There is too much copyrighted material that it is impossible to check for it.

The law states that the user posting the material, and not the website, is responsible for copyright infringement and or malicious activity. Drupal should protect itself and not do anything that puts us at risk of legal action. Telling someone that a project is safe, or free from copyrighted material is not only irresponsible, but it is also potentially putting Drupal responsible for any losses. Users are ultimately legal for their own actions.

Now if someone is found to be intentional in their malicious activity should be reported to law enforcement. Coders that violate copyright should be strictly dealt with, and their projects removed. A project that is poorly maintained, unworkable or harmfull should also be removed. These things will be reported either via the security team procedures or the review process and/or web masters queue.

Open source is secure because its code is visible to all. Everyone has to take responsibility for security when they use open source code. This transparent process is what makes it work. Just look at how many security issues and bugs are found by users as oppose to designated "reviewers". I bet that users find the vast majority of problem code. Also users are the ones that find the bugs. Check any issue queue, and they are not only full of reports of bugs, but also have quite a few patches.

Now if someone wants a higher level of security, they should higher someone or find an external company that can provide the resources to make sure that the modules they use are secure. That is why you pay Red Hat, Acquia, and so many other companies that rely on. Open Source is not free, its OPEN. It requires responsible users.

An open system works when you let it. If these open process did not work the idea of open source would of died a long time ago.

Why do we assume that the vast majority of new contributors are ignorant? This is hardly the case. They are all programmers, project managers, system analysts, graphic artists, students, etc. New contributors are in fact one of the most valued assets we have. New users have amazing insight on things. Some of the greatest solutions have come from new users who are seeing Drupal through a fresh set of eyes. Many come with experience in Multiple CMS's, and most know quite a bit about what they are doing. I learn more by helping others then I do on my own.

I am proposing that we ask our users to review our projects, they are the consumer and they have know what they need and what they are doing.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Number of Modules?

MGParisi's picture

I realized another flaw.

This process was implimented at the height of the D6 life cycle. Soon D6 will be phased out, and D7 will take its place. Just like with D5->D6, many modules will not have upgrade paths. The number of stable D7 modules is quite limited (this is about 4 months from the initial launch of D7). It took about a year for D6 to have enough stable modules for most people to upgrade. Unfortunately as I look at the time periods, these changes happened at the peek activity for Drupal Module development. It also took place at the peak time for D7 development.

Is it possible that the review process and the continual notion that the number of modules being implemented was large because of the life cycle of D6 was at its peek?


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Cyclical

MGParisi's picture

Security Team

Are the module related issues of the security team related to the release of a new core version. If so, wont we see a ton of security issues start to spring up? Probably will be around late fall, early winter when people start to make the mass switch. This will probably flatten the current security team with a ton of new issues. From what I have seen in looking at wiki timestamps and such, that it seemed allot of the activity started after I left. I saw it happen, no one was using D6, then suddenly everyone switched focus from D5 to D6. It happened quickly, within a month or two. I don't know what triggered it, but it fast.

If I was a betting man I would assume that there will be another trigger that turns everyone towards upgrading to d7. I would assume it will be around late fall. I left doing Drupal work around Christmas time so D At that time the security team, web masters queue and review committee will get smashed. It will then taper down.

From what I have researched, this started out because the security team was overwhelmed. The knee jerk reaction was to try and prevent errors through a review. It may have reduced the security team work load, but at what cost?

I would assume that what it actually did was reduce the over all issues by reducing the number of new modules, themes and coders. Maybe even reducing the over all number of coders all together. Also Eliminating security responsibility by removing modules out of "official support" is also questionable.

This is throwing the baby out with the bath water:(

I would love to solve the problems the security team has. And if I suspect, the security team will face similar problems in a few more months. Either that or there really is a death to the number of contributed modules.

IRC Channel Drupal -> Drupal-Contribute
There is a huge decrease in development and contribute talk going on. When I left Drupal, the channel was bouncing. Now there are period (even in the middle of the day) when nothing is said. I fear that maybe a result of less active development. I hate seeing people ask questions at certain parts of the day. I can almost hear the crickets. I wish I could help them in their questions, but the questions about GIT and about the API are beyond what I can answer:(


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Just re: IRC being quiet - I

arianek's picture

Just re: IRC being quiet - I think that the D7 launch definitely has allowed for some breathing space. But also, with the community constantly growing, I think a lot of people get their work done in other channels. Most major projects have their own channels at this point - which maybe is worrisome, as people might be less out of touch with what the various groups are working on. But this will be an ongoing change that we'll have to adapt to!

Responding to Jakub (meba)

pwolanin's picture

Responding to Jakub (meba) above - if there is crap in the queue, that's going to be a real disincentive for anyone to invest time reviewing.

Every person asking for review should be able to make a few (supported) claims like they ran their module through Coder and fixed critical flaws, that they ran a search for duplicate modules, and that they themselves assert it meets each of the checklist criteria. If this was true, the signal to noise would be better.

Frankly, anyone who asks for review and fails should be sent to the back of the line (e.g. wait at least a month before trying again). The review should just be validating some very minimal standards of quality and competence, so if you fail even once you are being a troll.

just popping in...

leisareichelt's picture

I'm loving this discussion, and although I've got nothing to contribute on the 'how review happens' side of things, I have made a big note that we need to make sure this method of contribution is much more visible on the site so that the people who would be interested in contributing in this way know how they can get involved.

to that end, can we make sure that whatever the process we end up deciding on (and let's do get to a decision on this) is documented and I'll make sure it's easily found by people who might be interested in it.

I would also find it tremendously useful if we could document the user flows/journeys for:
a) people who have work that they want reviewed.
b) people who want to review some work.

being able to see what they are now vs what we want them to be in a step by step (perhaps even visual) format would be incredibly useful.

leisa reichelt - disambiguity.com
@leisa

Is this about "status" and "queue order"?

MichaelCole's picture

Hi, bit new here but,

This seems like two problems:

1) Status. "Sandbox" projects are perceived as less cool that "Full" projects. This is a "sales" problem for the sandbox feature.
2) Review queue order.

What about recycling the "Reviewed & Tested By the Community" concept from the issue queues?

1) Projects pages get badges for positive reviews
- Image badges for teams: "Security Team", "Documentation", "Code Quality", "Coder Module Bot", "Summer of Code", whatever.
- Text badges for individuals: Drupal.org usernames, with links to the user's review.
- How does an integrator know a module has been reviewed? Check for the badge.

2) Remove the "Promote to full project" link for sandbox projects until they have some mix of badges. This will trim the "full project" queue substantially.

3) Teams can order their review queues based on the number of individual reviews, and "View Usage Statistics" data.
- Want your project to be higher up in the queue? Get a peer review.
- Want your project to get a peer review? Offer to exchange reviews with someone else in the queue.

4) User profiles show which projects a user has reviewed as an acknowledgement of work done reviewing.

5) Projects (sandbox and existing projects) qualify for "search status" based on their badges.

This let's:

  • Integrators search for modules based on "search status" - without having to do reviews themselves (for lack of time or skill).

  • Developers improve their code through peer reviews, and improve their projects "status" through team reviews.

  • Reviewers focus on modules that are active and interesting. They can follow the integrator herd, and active new development projects.

That takes care of "queue order". For "status", that's selling the fact that all projects (sandbox and project-paged) compete for "search status" on the same playing field. The only thing "Full Projectness" provides is a clean URL in the "/project/*" namespace.

I'd be interested in helping to code this project, if it's wanted by the community and accepted by the website team.

Mike

Jury Duty

mikeytown2's picture

Ask someone who is already a maintainer to go and checkout an application. Have it choose a already approved user randomly and never more then once every 2 months. Make it similar to Jury Duty in short. The other thing that would help is auto running it though the coder module; allow the applicant to either fix or mark it as OK with a note for each warning generated by it.

.

Michelle's picture

Up to once every 2 months randomly pick a volunteer and tell them they've been volunteered for something else? Yeah... I don't see that going over very well...

Michelle

add a review system to drupal

lpalgarvio's picture

add a review system to drupal where the reviewers can analyse the code for standards/bugs and then mark it as safe, secure, or trusted.

same thing for a developer who passes say, 3 module reviews.

Luís Pedro Algarvio
Drupal and DevOps Developer, Evangelist & Trainer
lp.algarvio.org

It is pretty clear the

giorgio79's picture

It is pretty clear the current review process does not work due to lack of manpower
Another possible solution:

  1. Do what Firefox does on the addons site, let devs create projects, show them among modules so users can install it, but display an "Experimental" badge on it. Once a module has, say 100 installs, a review is taken in place, and the experimental or unreviewed sign is taken off...If a module has 100 installs they must be doing sg right.

  2. Bring in more automation with a qa bot like approach currently for drupal core...Scan for copryright notices and non gpl licenses etc etc. Require developers to add simple tests so a bot can install it and test it out...

I am also not a big fan of CVS, GIT and all these. They are ok as backend, but in the frontend why dont we have a simple file upload field where we can upload zip files. Comment on this feature request, if you want to simplify committing code:
http://groups.drupal.org/node/141069

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

This makes the most sense to

acouch's picture

This makes the most sense to me.

I agree, just remove the module approval process

Pasqualle's picture

why do we need to review before creating a full-fledged project? I only review before I want to use it. why should I review something I am not interested in? That's why I do not review new projects..

  • just move from sandbox to incubation, when the module creator feels like it (and if he/she do not have the "publish new projects" permission).
  • modules in incubation should display a nicely composed warning message on their project page.
  • users who can publish new projects can thumb up or down modules in incubation. 10+ could give the new contributor the "publish new projects" permission (it means the module has at least 10 valuable users). problem solved.
  • and every new code contributor is a new reviewer.

and I think, all new modules should start in incubation, to have usable and more secure modules on d.o, but it is another issue..

I will also note that a lot

Dave Reid's picture

I will also note that a lot of the 'stuck' applications are modules that integrate with 3rd party systems, and it is a pretty big hurdle for someone to properly review it since the reviewer should be able to enable the module on a site and test it.

Senior Drupal Developer for Lullabot | www.davereid.net | @davereid

Weighing in

zzolo's picture

Wow, this issue exploded in such a short time. I am just noticing it. I would like to offer my thoughts, as probably one of the few people that has a pretty thorough view of this process and area in the Drupal.org community. I have been drafting my thoughts about this since DrupalCon Chicago, but my time has been pretty limited.

@MGParisi, thank you for bringing this up. Your frustrations are very understandable. There are serious issues with this process. I do not agree with the approach of just opening the gate, as we discussed in IRC and what I'll lay out below. But, again, I very much agree there is a problem and it has some very negative impacts on our community.

Please note that I use "code review process" and "full application process" interchangeably, but the latter actually includes more than just code review (ie co-maintainership).

Current Resources

First, I would like to point out current major resources (and subpages) that exist around this topic as there is not too many links to these in this discussion. There should be a lot more, but I will go into that further down:

  • Instructions on How To Review: This is the current complete documentation on how to review applications for reviewers. I wrote this based on my experiences of doing reviews and describing what should be done. This is found in the Contribute section of the handbook, which has lots of great links and there is no reason to push the review one up.
  • Full Project Documentation: Lots of docs about the full project application process for reviewees.
  • Code Review Group: New group meant to be a place for people doing Full Project Applications reviews.
  • Peer Review Group: Been around a bit, focused more on any peer review. This group is not technically the place to have this discussion (understandable to not realize that, see comments below).

Leadership and Team

A lot of things in the Drupal community, especially things like this review process, don't have a leadership and team members behind it. Specifically there is no formalized team around this. This would be some clear leaders and people that are responsible and then members that are mostly responsible for making things happen (doing reviews).

A lot of people may pull the Drupal "do-acracy" card here, but that works on a very small level; it will not get a culture shift through or help organize a large group of people to do something. A great example of this is the Documentation Team. The Docs Team has:

  • Clear leadership
  • Known members
  • Known places for discussion about this
  • Regular meetings

These things need to happen before any of this discussion goes further, IMO. If we cannot work on creating a base team to address this issue (and in the mean time, actually do reviews), then we will end up in a discussion like this over and over again.

Formalize Process

After some initial leadership and team building described above, the next thing that needs to happen is to formalize the review process. Yes, this sounds all bureaucratic, but its actually simple and needs to happen to be able to have meaningful discussions about the process and to be able to change it in a sane manner (not some long thread that has the word KILL in the title :). This includes things like the following, which I will go into some more detail on some below:

  • The goals of the process. This is actually the most important thing, IMO, and without it, a discussion like this happens with no real ability to address problems.
  • The very basic steps of the process (and the outcomes of these steps)
  • The audience of the process. This is very important as it really defines how the review needs to happen.
  • Review of process. This needs to be something like, meet every 6 months to discuss what needs to change.
  • Collection of metrics for making reviews and alterations more productive.

We should also be doing this around team building and some other aspects of a code review team.

Goals

What are the goals of the full project application process. Some people might just jump to "allow people to have git vetted status", and though this may be a valid answer, I don't think it's actually complete enough to create a meaningful experience for the Drupal community as a whole.

When writing the How to review page, I tried to lay out what I thought the goals of the process were given @webchick's talk and my personal experiences with the process, and have stuck with those goals. I will reiterate the short version below:

  • Licensing
  • Module duplication
  • Security
  • Best practices and coding standards
  • Mentoring

The last two are the points that most people seem to have issue with. They are more subjective then the others, but that doesn't make them less important or less mangeable in the process.

Concerning best practices and coding standards. These are extremely important in our community as it allows us to all be on a similar page when working on code together. These are crucial to collaboration in a community of this size, which is the heart of Drupal and open source.

I think this is usually where people bring out the the "well, when I went through this process, X happened and it took Y long". this is really not a valid argument for two reasons. 1) We have no formal process to say, well, that should not have happened; we just have opinions. And 2) if you are describing anything that happened more than 2 years ago, our community of coders, modules, issues, etc was half it's size; our processes have to change with growth.

It's this growth that I think the mentoring aspect of this is really important. We have such a big community; and for such a big community we don't really have a lot really strong formalized groups that do mentoring. This means that it takes a long time for people to traverse this crazy, awesome beast that is Drupal. Let alone the learning curve of the actual code, learning how to smoothly contribute to Drupal as a community is really, really difficult (I am still figuring things out after 4+ years). this is why mentoring is so important.

On a personal note, a lot of the reviews I have done, even after taking awhile, usually end with the reviewee thanking me to take the time to teach them about Drupal.

Identifying Problems

We need a better way of calling out problems with the process of full project applications. Yes, reviews take a long time, we all know this. But, what are the really problems here. I would suggest some of the following, though I don't agree with all of them. But please note, that even trying to identify these without the above steps, is a very fruitless tasks, and again, ends in discussions like these.

  • There are not enough reviewers. This could involve:
    • People don't like to review.
    • People don't know how to review.
    • People don't know they can review.
    • People don't know value reviewing.
  • There are too many applicants. Yes, this is a problem. Here are some possible reasons for it.
    • People stumble on to it.
    • People don't know the difference between a full project and sandbox (ie might not need full project).
  • Review process takes a long time. Again, this is in the perspective of the goals above.
    • There is no mentoring happening in the sandboxes.
    • Reviewers are not confident enough.
    • There are not enough people with Git admin access.
  • And I am sure there are others...

These are all questions that need data and discussion to answer.

Solving Problems

So, after identifying problems, how do we solve them? Again, this is all built on the above steps, and just an idea of what it should look like, not actual answers. Also, solutions need to be tried and then reviewed - an iterative process.

I would argue, very strongly, that these problems don't warrant just opening up Full Project access to anyone without a small review process. Yes, these are serious issues, but there are multiple ways to solve them. This could include some of the following:

  • Connecting community values and goals.
    • If these are the goals we decide on, then we need to really push this as what the community already needs and values.
    • We do value coding standards, a lot. Check out the docs on it, they are awesome.
    • We do value mentoring, but we don't necessarily facilitate it.
  • Clear leadership and responsibility.
    • It needs to be obvious who is responsible and what that responsibility is.
  • Team building around reviewers.
    • Code review sprints like the one I led at DrupalCon Chicago or the one in Portland next month.
    • The code review group needs to be more supportive of its members.
    • Regular meetings
  • Showing value of reviewers
    • Highlighting members more. There is a profile check box for "I reviewed...". Maybe badges.
  • Showing value of reviews
    • Highlighted stories of reviewees.
    • Case study of a reviewee that went on to be a rock star.
  • Automated reviewing
    • This is the huge technical improvement that should happen.
    • Automated Coder reviews.
    • Check for images or other possible non-GPL files
    • Is there a README.txt
  • Tell sandboxers more about the process
    • It needs to be more obvious what a sandbox is and what it isn't
  • Promote peer review in sandboxes
    • If review actually happened in sandboxes, like it was the goal of sandboxes, then full project reviewing would be easier.
  • And I am sure there are many more.

My Involvement

A lot of what I described above is kind of against the usual "do-acracy" of Drupal, or at least it may be perceived that way. There are structures like this in place already in Drupal, so it really is nothing new; they can be found in the Docs Team, Core Initiatives team, Security team, etc.

I have been slowly working towards these things over the past year, but it is slow and my time is limited. This is my first real written vision out to the community. I do believe in this process, and have made it known in multiple places as far as what I value in the process. To me it's all about mentoring and getting the Drupal community to value it. I don't like arbitrary gates for people to go through, I just want to maintain and improve the quality of our community.

But, I really don't think this process will ever become better without doing these initial steps (or something similar). I also personally won't put much of my time into this in the future unless something like the above happens (with my own personal help of course). This is not a "I am taking my toys and going home" thing, I just personally watched over the past couple years of some great improvements happen, but many more discussions like this that are more harmful than good.

We cannot fix this overnight. It is a serious problem, and we are indeed, on some level, hindering Drupal's success and growth. That doesn't mean that doing anything necessary to fix it tomorrow is a positive thing for the community in the long run.

(This is a rough draft, that took hours, but I will be cleaning up and reposting on my blog and on the code review group.)

--
zzolo

Success Measurement

MGParisi's picture

Im VERY glad to hear from you ZZolo, and have been waiting for your response:) I love your post.

I disagree with your decision to continue the review process but thats ok. I am willing to stand behind ANYONE who can get this moving foreword. If needed I would appreciate it if Dries or Webchick steps in and assigns leadership.

Like you said, a Do-ocracy works for simple tasks, but to accomplish something big we really do need a set of people who can give the go ahead on projects outside of core. We need a "Benevolent Dictator" that can make the decisions where no decision can be reached made. If people are unwilling to compromise and come up with a solution, the Dictator steps in and makes the decision.

Quality
I think you are hitting the nail on the head. The only thing I would like to see is a set of criteria to measure success/failure. This will probably include any statistics, opinions, etc... Then a review process team agrees that in 6 months you will review the process and make improvements. Document this process and continue with a yearly, bi-yearly or quarterly review. This seems like allot of work, but it really isn't. The idea is to keep improving the process for better quality. It should be rare that a complex process (like this) goes through these reviews without changes.

I have been doing these systems for a long time, and have setup ALLOT of process quality reviews, and usually even the system analysts that create the process reviews usually have process's they follow. Its quite amazing at how much resistance there is when you first purpose something like this, but once in place, the results and ease quickly quiet's any skeptics.

My Support
I may disagree with ZZolo's decision, but if he is given the authority to break the stale mate and make the changes then I would be happy to work within his decision. I would be happy to help out with the process improvement so that the system works in the end.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Everyone on this thread, join in!

mlncn's picture

Short version: While there's a backlog, we will not be adopting all best practices. Dive in and review one or two and i can give the applicant promotion permission. Then, help craft a better approach-- which means to me at minimum making it easy for multiple people to check off different parts of one application review and making this process not the only time we flex our code review muscles.

While i was missing out on this thread i took care of or advanced a dozen applications. While we still have a five week plus backlog, the first priority is getting that down - and with the energy on this thread, we certainly can. MGParisi, you too-- or simply round up and recruit people, you are good at getting things moving :-)

I am now one of the few (who? what's our process for getting there? we should document these things!) with the power to give people full project permissions.

Get involved with reviewing applications. Anybody at all can check licensing and for duplicates (not easy to do for certain, but you can do it). Really anyone at all can follow the coding standards checklist also, which counts as a partial security review. Install the module and see if it works– invaluable! (Infrastructure point: Tarballs for sandbox projects would help here. Is there an issue for that?)

In other words, with no knowledge of module development and you can just about fully review a project application. And you will learn something about it in the process. And you will be much more informed to shape the review process, zzolo, will this be on a wiki / widely editable doc somewhere?

And at this stage, a review that provides reason to believe an applicant will be a conscientious maintainer and community member will mean

And with your experience reviewing, please help shape the more ideal process going forward. I have two thoughts on that at the moment— one, by separating the elements of the review as zzolo has done, anyone can come along and say they've approved parts A and B of the process (say licensing and duplicate code) and someone else can handle the rest. Two, separate the concept of review from the application process in the sense that this is not the only time that happens, http://groups.drupal.org/peer-review

(Also, i continue to put forth that any process that applies only to new developers and not to 'vetted' ones will inevitably be separate and unequal, but hey. In the meantime i try to contribute to the process we have.)

benjamin, agaric

Some background on sandboxes:

eliza411's picture

Some background on sandboxes: it was an explicit requirement during the migration that unsuspecting non-coding Drupal users not accidentally be able to get sandbox/experimental code, thus no packaging. I suspect any change that would make packaged versions of sandbox code easily available will require a long discussion in its own right.

That said, Gitweb, the current repository viewer provides snapshot tarballs. From a project page, click the repository viewer, then click snapshot:http://www.diigo.com/item/image/17mlr/7jkh

Unintended Consequences

MGParisi's picture

I can look at this entire thread, know all of the various issues, build the system in My head and see what it will and wont accomplish.

Ultimately success would be declared because those who have patience enough to put up with the process will be good Drupal contributors, but a good fraction of those alienated people would of been good contributors.

I also feel that by implementing this process we are unintentionally implementing a way to eliminate personality based diversity. I do not know what overall impact this will have on the community, nor what specific benefits this elimination would lead to, but I suspect impatient, the easily frustrated, or the other personality characteristics do have allot to offer. I guess Allot of people with ADD are going to be working on Joomla, lol... (Then again people with ADD are WAY too smart to use Joomla). We also wont get their feedback because again, people who don't like the process will rather leave then start conflict.

I would assume some wont learn through the mentoring process and would instead learn better through interaction with users of that module. Some will have a difficult time describing or selling their modules, or well get mentors that over step the boundaries of those they are mentoring. Their is no process for how to handle disputes with mentors, and most people when faced with a conflict will tail it and run before they say anything. I know that I get ideas in My head and I have a very hard time explaining them to others (like why writing down the processes we use provides a huge benefit).

The longer we implement this, the more contributors we will have that has gone through this process. Thus we will have less people who object to the process and the voice of the minority will die (or be drowned out).

We will get a specific type of programmer, and some people who use other methodologies would find the process difficult. As sun says, Drupal code development is very "agile", so those that use a more heavy style will feel extremely uncomfortable (I know I do). Or as I pointed out, people who tend to not care much about coding standards and instead focus on Rapid Application Development, or Focused (non flexible) Performance based application development. In fact, if I wanted to build a light weight version of a module then My Module would be marked as a duplicate even though its light weight is an advantage in performance (but offers less flexibility).

Not to mention that their is about a billion and 2 ways in which a malicious person could hide embed code, or how the process for adopting an abandoned module or becoming a co-maintainer is different and easier then going through the mentoring process.

Now if only I could take all of this stuff in My head and somehow put it down in a way that will give it justice. But I think I have done the best I can:( Sorry, I'm not the best communicator. So if you feel that a system is fundamentally flawed, but the consensus or leadership disagrees with you, do you try to make the best out of a bad situation by working within the restraints set upon you or do you simply walk away from the entire process all together?


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

love it, change it, or leave

rolf vreijdenberger's picture

love it, change it, or leave it comes to mind.

some people love it, if you don't love it, you can change it. if that doesn't work (and that's what this discussion is about), leave it.

that essentially, is what will happen to some extent.

how can you keep the 'leavers' by changing it to something that's more workable?

Kind regards,

Rolf Vreijdenberger

drupal and flash: http://www.dpdk.nl/opensource/drupalproxy-as-a-bridge-between-flash-as3-...

Comment From One In The Process

mermentau's picture

My module has been in the application process since February 26th and I have a couple of thoughts.

There are some things that I appreciate about the process. I like holding to the code standards and the mentoring that I have received. I have found that anyone can review using the resources that zzolo linked. I have done one that has been marked "fixed" and have got another underway. Pay it forward as some here have suggested works for me. The process has helped me get comfy with the IRC which had intimidated me a little before. I feel at ease there now even if I don't have too much to say.

A problem exists to me in that people submit applications without a clue as to what is required. I mean we should not have to tell people to put a link to their sandbox, use coder module, no 3rd party coder rules etc. This is information that's easily available and again I think zzolo's links above probably cover most everything.

My suggestion would be let people take a test prior to being able to submit their project for review. A simple online test that would be easily passed if they had read all the "required reading". Open book is fine since the point is to get people to look up the basic requirements for submitting a project and having it approved. A lot of reviewer time is wasted telling people what they should have already read.

I really think that if someone has read the required info and their module works as described review would be pretty straight forward. What's the point of making it too hard if they can turn out junk once they are vetted. When I go shopping for a module on D.O. I'm amazed at the abandoned modules out there. Some never got to the point of having code to look at and they are there for years.

To me we need a Drupal Approved badge for modules, but that approval process would be another issue. One that would be applied to all modules that already exist.

Great ideas here

ccardea's picture
  • +1 vote for having some process to make sure that people know the basic requirements before submitting an application. That could eliminate a lot of wasted reviewer time.
  • +1 vote for some type of approval process that weeds out old abandoned modules.

Tests would be great

fizk's picture

+1 for taking a test prior to being able to submit their project for review.

"Teams" in open source

greggles's picture

So..I agree with most everything you've said, but...

A lot of people may pull the Drupal "do-acracy" card here, but that works on a very small level; it will not get a culture shift through or help organize a large group of people to do something.

The examples you point to are actually very small. Consider our largest team: core contributors. There are 950 of those folks and none of them call it a team. I think size is unrelated to the need for team vs. individual do-ocracy.

The thing about teams in open source is that they often don't work.

The security team is a team because they deal with issues that by their very nature need to be private. But the membership changes a lot and, to be honest, the team is often dysfunctional.

Here's my sense of what makes teams work, the pitfalls, and how we can make them work for this effort:

  • Teams can be great as a way to give users a target for volunteering. Lots of folks want to help but don't know how. A team gives them a "how" and "who to ask if you have a question." But having great docs (which you're writing now) and good welcoming processes are just as valuable.
  • Teams can be useful for recognition. However, lots of contributors don't like recognition (it feels like a leaderboard and leaderboards feel icky to some folks). On the flip side of that, "team leaders" and "team members" often join because they like recognition but then don't actually do work and that is demotivating for the "unofficial but hardworking" folks.

My main request: Keep the team list dynamic. We announce a team and we'll get 30 people who sign up and 3 people who do work. That's great if it's 3 more workers than we had before. But we must make sure any team list reflects reality, is updated regularly, that it's clear how to join (do reviews) and that new users are welcomed quickly based on their efforts.

A few suggestions

ccardea's picture

@zzolo

I appreciate that you have put a lot of time and effort into this post. These are just a few quick thoughts after skimming through this thread.

  • I think its been mentioned a couple of times already but I would add to your goals to try to establish that the individual seeking full project status will be a good maintainer of the project.
  • If there is a shortage of manpower, then there needs to be a way to focus available resources on the most important projects.

Allowing some kind of release before achieving full project status would help achieve both of these goals. Doing so harnesses the built in power of the community and it actually is foolish not to use that. The users will tell you if the project is working as advertised, and the project owner's responses to issues will let you know what kind of a maintainer he/she is likely to be. If a project is attracting attention and users, and does not have a lot of unresolved issues, that should add weight to the project's importance in the review process. It seems to me that some type of early release program could be incorporated into the application process.

It's already been pointed out that a project author can always put his project on github, if he/she wants other people to have access, but still there needs to be some indication that people are using it and that the maintainer is maintaining it.

Having listened to webchick's talk on the subject, I understand that module duplication is perceived to be a real problem on drupal, and one of the prime reasons for the project review process.

vidhatanand's picture

Complete removal of module approval process will make things a mess. Once its removed, in one year you will see 5 modules for same purpose. It will become a hell to choose the best one of them.
So lets retain it as it is. But invite more code reviewers. Like just emailing all guys who have more then 3 approved modules to come and help by reviewing some from issue queue. I think the prob is most of people who are capable of doing so are not being involved too much. They are not involved coz they even dont know that drupal is having this manpower prob and its hampering seriously.

Lets make some proper initiative by reaching them out actively.

lol, if everyone that posted

rolf vreijdenberger's picture

lol, if everyone that posted here took the time to review 1 module, there would be hardly any queue left :)

I agree with comments that say there is a responsibility for people that apply (including myself at the moment) to make sure they have done their work.

clean coding, understanding what they are doing (which might mean them getting more involved in a group, getting suggestions from others, having tested it themselves in scenarios, etc), running the code through coder and cleaning up all there is to clean up, documentation being in place, having some iterations in their sandbox project etc.

This would mean that there would be less crap to review for the reviewers.
I just read the description on a sandbox project that said: "It's not entirely finished and there are still some bugs, but it can already be used." in a three line description on the sandbox!! When I read that, even I was not motivated ("crap") and I'm not even a reviewer. I informed him about this.
But the crap thing, that should be addressed to say the least..

At the moment I'm waiting for someone to take a look at my first module ( an amfserver for flash/drupal integration in the services module for D7), but I am an experienced developer, am very familiar with the field I developed the module for, am very very obsessed with making it bugfree and stable and have made sure this thing is rock solid. I even said so in my application etc. but no one responds or even gives feedback that it's even noticed. Granted, it's only 3 days, but still I know that the module is as it should be, having gone through the troubles of extensive testing myself (and having included a testsuite).

Now, it's not bad, because at least the code is online and we can use it, but of course; everyone's module is the next best thing and we want to have it approved.

ALso, before applying for a project, won't it be good to just have some basic checkboxes for things that should have been done like:
- Coder review
- bugs fixed
- tested
- is there an audience for your module?
- peer review/friends' input
- level of experience as a coder
- drupal experience level
- comments in the code
- checked for similar projects
- well thought out page description (shouldn't this be just a fixed format??? everyone needs a 5 line description, a why as to using the module, roadmap, feautures, installation section)

and if you can't have a couple of those you can't even apply? That would really make life easier for reviewers

or maybe a better description of the standards that your project has to adhere to? It's easy to get to the project application form without having read the pages that lead to them. Make sure that it is read...

So while I'm not giving a solution here or pretending that I know a solution I hope this gives some insight from my perspective as a potential module writer.

anybody want to do a review on my module :P
application: http://drupal.org/node/1122200
project: http://drupal.org/sandbox/rolfvreijdenberger/1122068

cheers, good thread!

Kind regards,

Rolf Vreijdenberger

drupal and flash: http://www.dpdk.nl/opensource/drupalproxy-as-a-bridge-between-flash-as3-...

in favor of removing the approval process

ankur's picture

+1 for removing the approval process.

The suggestion that everyone capable pitch in and help review is time wasted on bureaucracy, time that is better spent working on code.

Also, Drupal already has a review process: it's called whether or not people are using your code. The best peer review is whether or not other developers and users are making use of your module and the comments/bugs/patches they submit back to your issue queue.

For people worried about more than one module doing the same thing and not being able to discern which one is better, here's your chance to help: try the different modules and and promote the one that works best for you.

If there really is a need to figure out which modules have a certain level of trustworthiness, then I would suggest, at most, some kind of "badge" or "certificate" for the module (as suggested by someone else previously in this thread), or perhaps a way to float the more commonly used modules to the top of lists when people are searching and filtering through module lists.

So, to summarize, let the community moderate the modules in the way that they already have been doing so for years, as It is the method least prone to politics and bureaucracy. To make it easier for people to find the maturer projects, have a way on the site that uses usage statistics and other forms of organic community feedback to make them more visible than the poorly implemented, random, one-off modules.

Reviews are valuable

sreynen's picture

The suggestion that everyone capable pitch in and help review is time wasted on bureaucracy, time that is better spent working on code.

I completely disagree. Time spent reviewing is working on code. It's just someone else's code. Collaboration is a Drupal principle and reviews, done well, are a great form of collaboration. I actually think it would be great if we could go the other way and do more reviews of existing projects. Sure, it takes time, but it's time well spent.

pair programming

rolf vreijdenberger's picture

time spent reviewing is working on code, well said. That's why pair programming works and very valuable in some situations

Kind regards,

Rolf Vreijdenberger

drupal and flash: http://www.dpdk.nl/opensource/drupalproxy-as-a-bridge-between-flash-as3-...

no disagreement here

ankur's picture

@sreynen

I don't disagree that reviewing code is a good use of time, only that it is more fair and effective through a project's issue queue than through an additional approval process. The approval process forces it to happen even if there aren't the resources for it. The issue queue allows it to happen naturally. Collaboration is a principle to be encouraged, not to be enforced.

@ankur Very well said!

giorgio79's picture

@ankur
Very well said! Totally agree with you.

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

Let us categorize modules

kevinsiji's picture

Modules should have some growing up stages. Let us call them, 1. Just born, 2. Child, 3. Young Adult, 4. Matured.

Just Born: Any new module from any contributor. Minimum requirements (like GNU/GPL, Read me, etc.). A peer review happens here by the fellow Drupalers and get a nod of approval that its Working as claimed in Read me.

Child: Here the Module Review Team checks in and do their initial job. Once the Review team approves, It becomes a Young Adult.

Young Adult: The Security Review Team go for a thorough security check and the module get a PASS, it will get the status Matured.

Matured: This module carries all the guarantee of having followed Coding Standards and no Security Holes.

This will solve the issue of waiting to get the module published.
Also, the coding standards, security, etc are taken care in their respective planes, and Module users can rely on the modules they take from those approved planes.

My one cent. :)

kevinsiji

So I got back to this,

meba's picture

So I got back to this, thinking what's the fuss about reviews, their complexity and how it's killing Drupal (no offense to anyone!).

I reviewed approximately 10 applications today and sent around 7 of them straight to postponed or needs work due to multiple issues, usually:
- Security issues
- Not seeing code at all
- License issues

I did this with the OLDEST applications in the queue. My 2 conclusions:
- Review process IS needed
- I didn't check the newest applications as well but it might show that there is a reason why those applications were so long without an answer?

Now, do not jump on me with the solution - I am not trying to suggest a solution (not in this comment), I am just telling the fact that I observed.

My $0.02 :)

A Little Organization Would Be Nice

mermentau's picture

In such a rigid technical environment would it be to much to hope for some organization in the ordering of reviews. It seems clearly at the whim of the reviewers. You chose the oldest and others use their own method. The result is that some get done in under a week and others languish for months. I see applicants begging for reviews or just to have a time frame revealed.

A lot depends on your luck in who does your review. Some reviewers are exacting and others seem rather easy. After 2 months I keep telling myself not to be impatient.

I also reviewed about 15

Dave Reid's picture

I also reviewed about 15 applications in the last two days (approved a couple!) and found lots of licensing issues, no code, or security issues.

I'd like to share my personal review process in case it helps anyone out. It takes me about 5 minutes on average.

  1. View the git repo. Can you at least understand what is going on in the code without too much difficulty? If not, nicely link to the coding standards and ask the application to download Coder and run it against their project.
  2. Do a project search for similar modules - see if it directly duplicates anything, or if it close enough to a similar project that it could very possibly be a feature request to an existing project. Ask applicant if they have asked the maintainers of project X if they would consider new feature Y and could coordinate.
  3. Look for licensing issues. Check for non-GPLv2 files or external libraries. Neither are allowed.
  4. Look for red flags - outputting variables directly into HTML. Use of $_GET, $_POST or $_REQUEST all get checked. See if database queries are using correct APIs. You should and need to be able to scan for the common security mistakes.
  5. Actually git clone the code, enable, click test, disable, and uninstall. I also like to type in strings like <script>alert('XSS!');</script> into any input the module offers to test for XSS. Note, this is the tough step for complicated or integrating modules, so I usually leave skip this and spend more time doing #3's 'sniff test'.
  6. Approve application! if 1-5 are good.

Senior Drupal Developer for Lullabot | www.davereid.net | @davereid

Code Review Group now a community portal

zzolo's picture

My vision stubbed out on the Code Review group. A lot of wiki pages have been created but content is needed. Feel free to add to it.
http://groups.drupal.org/code-review

--
zzolo

I started adding my thoughts

jthorson's picture

I started adding my thoughts to this thread, but it turned into a major post ... posted in the "Reviewee's Stories" section of the Code Review group instead.

Rethinking Early Release

ccardea's picture

It really helps to know what you're talking about before you speak. I was just looking around and found out for the first time that you can find sandbox projects through search and you can get the code through git. This changes my opinion on this issue quite a bit. It seems that the existing system already provides an excellent solution for making sandbox code available to interested parties, while at the same time keeping it out of the hands of the less experienced.

I would think...

MGParisi's picture

Doesn't this re-enforce the idea that sand box projects are rather hidden hard to get and go pretty much unknown to most?


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Re "teams"

arianek's picture

I think zzolo's vision for a more clear process and a Project/code review team is right on the money.

The only reason that Docs "team" is a team and gets larger chunks of work done are because there is a bit of a structure to work within. Otherwise, I feel that a lot of the issues would stagnate permanently (as they did while team lead handoff was in limbo).

Why I think this would work well for code/project reviews:

1) It's an open team. Anyone can participate in reviews without having to jump through hoops.
2) Like Benjamin (mlncn), anyone can apply to be an "admin" of the team, ie. have special privileges, such as being allowed to promote sandbox project to full projects.
3) There is a single issue queue for the team to work in.
4) The team (now) has a g.d.o group.

With Docs Team it's still very much both the team and the team leads that do the work - but the thing is that having team leads really keeps things rolling, since there are a couple people who really know the bigger picture as well as any best practices. If there are a couple people who would want to take charge here and work on the proposal zzolo has written up, I think that just a little bit of regular team building and promotion of what needs doing within the community, being a resource for reviewers to look to, and then having final promotion privileges would go a long way to get this all working more smoothly.

Warning, blatant begging lies within...

dreamleaf's picture

Just found this thread, and I really like the discussion. I have a theme sat in the queue at the moment (since April 26th). There have been submissions there a lot longer than mine, and I've watched others speed through really quickly. All the while, I've waited patiently, mainly as there isn't an alternative option :D

Despite 100% loathing the wait, I fully appreciate the review process and the need for it - maybe not in it's current form (as that really isn't working that well, due to manpower).

It was echoed a couple of times in this thread that you don't have to be "established" to review, it's also been countered that a good reviewer should really know what they don't know... new subs don't have this knowledge (hence the review...). For me that is why I haven't launched into even doing a partial review.. as I want the code on Drupal to be good, safe and in the hands of people who want to see a process through.

However, my theme is at http://drupal.org/node/1138978 if there are any lovely capable people wanting to knock the list down. I did mention there would be begging and also that I don't like waiting.

I do however, plan to jump in wherever possible to help on the review side ONCE I get something actually through a review.

No need to wait!

bonobo's picture

Hello, dreamleaf,

RE:

I do however, plan to jump in wherever possible to help on the review side ONCE I get something actually through a review.

No need to wait!

You can use the steps here to get started: http://groups.drupal.org/node/115554

Or, grab a look through this handbook page: http://drupal.org/node/894256

Are you active in your local Drupal Users Group? Have a meeting dedicated to reviewing project applications. We just did that in Portland and it was awesome.

If every local users group dedicated 1-2 meetings a year to code reviews of project applications, the queue would be non-existent, and the community could actually move on to doing more detailed code reviews of contrib modules.

But there is no need to wait - if you can create a theme, you can review a theme. The process of reviewing code helps people get experience as well.

Cheers,

Bill

Not Broken

pmagunia's picture

I have been in the review process for about 5 weeks now. I think if there wasn't a wait, a lot of projects would get approved which eventually get abandoned.

My reviewer has been giving me good recommendations about improving the project.

So far no problems.

Ongoing "approval process" for existing modules, please.

Ryan Weal's picture

Hello everyone,

I stumbled upon this thread after following links posted to my Project Application issue and I'm going to try my best to offer some solutions here.

As an applicant, I held back posting until the new git system was in place. I was really excited about it and I still am. My application has been in the queue for a long while but I'm still busy working on Drupal modules so I can say that for myself it has not been a deal breaker for me. I'm all for quality software so I can deal with a bit of a wait.

What I found most disparaging about the process is that I've taken great effort to have perfectly clean code but when I run some "well known" modules from drupal.org through the coder module I sometimes get stunningly bad results. In one example the number of errors was in the hundreds, possibly thousands. I had to kill my web browser because I was getting so many errors with one module. I looked at the code and I'm not surprised it failed so badly... it looked terrible.

I think what I'm trying to say in this posting is that we need a review system that is ongoing if we have these krusty old modules that were approved long ago and grandfathered to infinity sitting next to higher quality code (side note: thank you all for the history lesson on this). Popularity contests are not good enough for this in all cases because sometimes there is only one option so bad code can still get to be really popular.

I'm going to go out on a limb here and suggest some sort of block on each project page showing the result of an automated scan by the coder module. Number of critical issues, number of warnings. Just like we see with the "human issues" in our issue queue summaries. For each current recommended release.

Added bonus for this approach is that time spent in the applications queue will be reduced as I see a lot of "please run this through coder" messages in many of the issues.

Yes, in true Drupal spirit I'm willing to contribute to this crazy coder idea. Please contact me through the contact form if you can contribute knowledge or resources to kick this off (assuming people support the idea).

There is already the facility

catch's picture

There is already the facility to run branch tests with the test bot, see http://drupal.org/node/689990

The test bot has some level of integration with coder already.

The missing piece of that is a drupalorg module block (and possible code on qa.drupal.org) that could show the results of branch test runs publicly.

Here's the issue for

catch's picture

Here's the issue for simpletests - http://drupal.org/node/694876

That's a good place to start since it's already in place.

I don't know the status of coder module integration/reporting, but whatever we do for test results will be very similar.

Awesome

Ryan Weal's picture

Thanks, greatly appreciated.

Absolutely

fuzzy76's picture

I think this is a very good point! A thorough review process does not make much sense as long as you are free to do whatever you want from that point on.

I would actually suggest something even more radical. You should not be allowed to make releases before the underlying code passes through coder within a number of set conditions (let's say x number of warnings pr 1000 lines of code or something, zero can be difficult to achieve in some circumstances).

5 1/2 month review waiting time and counting

Mikey Bunny's picture

I posted my module for review now over 5 1/2 months ago. I'm pretty sure that it's ready to go in to alpha now but have been waiting over 5 weeks so far for the last reviewer to come back with any response to my last set of alterations for them.

I respond quickly to comments and request for modifications but now the process has become very tedious due to the waiting times. I have several other very useful modules that I'd also like to commit all held up by this process. I've even considered posting a much simpler module in the hopes that will get reviewed faster.

While I'm waiting I have taken up reviewing two other projects on the list. However, unlike me I find myself waiting several weeks for those module owners to come back with any response or updates. This makes the reviewers process tedious as well.

I agree 100% with a review process as there is a lot of badly written modules out there and the mentor process from the feedback I received is invaluable.

Anyway, "Honeytrap" module posted March 11, 2011 and still waiting in the review queue http://drupal.org/node/1089490, status Critical.

Broken

MGParisi's picture

How long does it take to see that this system is broken. Drupal 7 Module Development is so slow compared to Drupal's 6 cycle. I once again PLEAD that we change this to a volunteer based policy... Allot of current modules NOT in sandbox are broken with patches in issue queue that have been approved that no one is committing. Broken modules are all over the place, and the number of modules that have been converted but not committed is large. I wont publically post examples as some of the modules are from people who are have high karma and are very busy, while others simply do not need to be pooped on for not having the time.

Personally I have version access but I would love to have a mentor(s), however anyone who is willing to spend the time is probably reviewing modules. There are people (in other groups) also requesting mentor ship and receiving no offers. Id like a mentor who would be willing to try and mentor me and report on their experience in the Diversity group!

BTW, I see the movement to GitHub as a very prominent example of how Drupal is simply not meeting the module contrib members requirements. It saddens me to see this happen. This needs to be a critical priority as module development is the lifeblood of Drupal.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Processes

sreynen's picture

We have processes for voluntary peer review, finding co-maintainers, and dealing with abandoned projects. These seem more closely related to the problems you're suggesting.

Already being fixed...

Ryan Weal's picture

Hey there, I want to point out that much of your concerns are already being addressed by the code review group. There is a good summary of the suggestions here: http://groups.drupal.org/node/163549 and some of those suggestions are already live (see http://drupal.org/node/1180798).

As someone who posted higher up in this thread I also found the process cumbersome but I'm happy that my concerns were taken into consideration by the community.

I'm all for the mentorship model but of course we have to work within the volunteer nature of the Drupal community. I am hoping to contribute more to the process in the fall.

Current State of Drupal 7 modules

MGParisi's picture

The majority of modules not in the sandbox are broken. Most are in Alpha and some that are not are simply unstable or outright broken. There are actual patches sitting in the Queue many of which I have run into and have verified as working, yet none of these patches have been applied.

The truth is that the d7 sandbox projects are of HIGHER quality then the ones outside of the process. Sorry, but install Drupal 7 and start installing modules and you will find a significant number have some serous issues (like crashing all or parts of your site). This is not limited to some poorly maintained modules but include some of the very popular ones. Now of course views, panels, and other huge ones have issues and are being worked on daily. These are not the ones I am talking about.

Truth be told, with a few exceptions, I have been more happy with sandbox projects then with official modules. In fact even some of the Drupal modules (rules) have bugs in them. Now I bring this all back up because it has been many months since the first post, and I can give dozens of examples of where released modules break sites while sandbox projects remain relatively stable!

Sorry, but many module maintainers are on D6 and not upgrading to D7. Some will not even create dev branches, respond to requests for co-maintainer, or apply patches that will fix issues, or that will upgrade a module to D7. With that said, how do I present what I have found without actually naming the dozen or so modules that I have ran into that have crashed portions of My Drupal site without singling people out?

I have checked the action and maintained an eye and ear to the process. It is nice to see the work that has been done, but the problem remains that this process takes time, does not ensure a higher quality Drupal module and that with more time, things are getting worse not better. A Good Idea with a Solid theory, but one that fails in the real world.

Unfortunately, the whole process started to form without any documentation. The documentation should of been written prior to implementing process. If this had been done, then a failure and success criteria would of been written, and I am relatively sure that we would be failing to achieve our goals.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Absolutely agreeing with you,

patrickd's picture

Absolutely agreeing with you, drupals module database really needs some good cleanup.

Nothing to add, but hoping for change :(

Needs more evidence

sreynen's picture

You're making a lot of broad statements here without providing evidence to verify or even understand the scale of your claims. If I look at 100 modules and they all work, I may think 100% of modules work. If you look at a different 100 and none of them work, you might think 0% work. With over 8,000 modules, though, we're likely both pretty far off reality. I'm guessing you haven't yet looked at general numbers, and suggest you do that to make your argument more persuasive.

Some questions about your claims:

The majority of modules not in the sandbox are broken

What is your definition of "broken" here? How many modules meet that definition? How many do not? Did the maintainers of the broken modules go through the review process more or less often than the non-broken modules?

Most are in Alpha

How many modules are actually in alpha? How many are in beta? How many have stable releases? How many have no releases?

There are actual patches sitting in the Queue

What is the average wait time on RTBC issues? How has that changed over time?

The truth is that the d7 sandbox projects are of HIGHER quality then the ones outside of the process

How are you rating quality? How many full projects are from maintainers who went through reviews? How many sandbox projects have asked for reviews?

many module maintainers are on D6 and not upgrading to D7

How many?

how do I present what I have found without actually naming the dozen or so modules that I have ran into that have crashed portions of My Drupal site without singling people out?

Specific modules aren't very relevant, as they may just be anomalies. If there's a general trend, as you've suggested, we should be able to see it in the answers to the questions I asked above. Most of us only ever look at a very small number of modules, which makes it way too easy to get a skewed perception of the general trends.

Answers and Questions!

MGParisi's picture

I fear this discussion is again headed into an argument with no answer. Unfortunately the argument will remain until Dries steps in, or better yet, the association decides on an answer.

What is your definition of "broken" here? How many modules meet that definition? How many do not? Did the maintainers of the broken modules go through the review process more or less often than the non-broken modules?

I set the scope of "Broken" to presenting an error message that prevents execution of features. Sandbox projects are often written for the Drupal version they started in. Few are upgraded or ported versions, many are ground up projects. Most are simple in scope and thus easy to program. Some maybe covered by other more extensive modules that may take up valuable resources or other assets that a more custom module does not.

Most are in Alpha

Authors state that their module is in Dev status. Most modules remain in Dev status their entire life cycle. Alpha is a term I used to describe applications that are barely working or that have consistent and continual bugs. It is important that this label will appear to be a criticism of the module, its developer or the work done, when in fact it simply is a state of a valid development process. A major release maybe written, but it is often succeeded with bug fixes found only in the Dev version.

How many modules are actually in alpha? How many are in beta? How many have stable releases? How many have no releases?

Good question! Since we have a Review process and not a Quality Assessment team, there is no process, definition or other criteria in assigning a module as Alpha, Beta, Complete. The lack of such definitions means that quantifiable data is not available for assessment. This is one of the problems with this review process as it lacks the data to declare success or failure.

What is the average wait time on RTBC issues? How has that changed over time?

The average wait time for a patch to be committed varies by the developer and version of Drupal they are currently maintaining. If an Authors main development is in 6.x, module commit times for D6 will be short, while D7 seems to be infinite. Again, I can name examples, but I fear this will be seen as an attack to those module owners; and once more I did suggest a method in which I could share this information in a healthy way, but have not had received an answer.

How are you rating quality? How many full projects are from maintainers who went through reviews? How many sandbox projects have asked for reviews?

This is true by many of the statements above, and from personal experience. However sandbox projects are not in a perpetual state of unknown, unless the author decides to go through the lengthy review process. Authors who already have GIT access are exempt from this process. Modules that have already been accepted are also exempt from this process. The process is too cumbersome to handle new developers, adding a process to review existing modules will only make things worse. The lack of data and quality assessment is a major issue! Not being able to say X number of modules are in Y state prevents us (and ME) from fully understanding if a process of quality improvement is successfull or a failure.

There is a possible solution. Make the Review Team into a Quality assessment team. Harness the power and the sure numbers of the module users to participate in a non-obtrusive manor. I'm thinking a light weight version of the security team. If a issue is marked as critical; which means the problem causes Drupal to Crash for a certain length of time without an answer from the project maintainer, or with inaction, or with other users not providing solutions, then the module will be marked accordingly and a warning of the modules status will be displayed on the description page. The quality team will ultimately become backlogged, but the information within the issue queue will empower module consumers to make wise decisions. No issue queue means no information and a lack of empowerment.

How many?

Is there any data on the number of D7 modules verse D6 modules. Also how many modules where available at 8 months under d6? I suspect and remember D6 modules being ported at a much higher rate then we are currently seeing.

Specific modules aren't very relevant, as they may just be anomalies. If there's a general trend, as you've suggested, we should be able to see it in the answers to the questions I asked above. Most of us only ever look at a very small number of modules, which makes it way too easy to get a skewed perception of the general trends.

What I find interesting is that before I came across this issue, I never really realized the magnitude of the disconnect between theory and practice when encountering the human black box. The biggest lesson I have personally learned in this discussion is that solid theory is often dis joined with reality. Data collection, Success Parameters, and Quality procedures when implementing wide scale changes in large systems should be a priority BEFORE the process is implemented. This discussion started a whole different process of thinking when dealing with such systems, and I started seeing a hole host of things in a different light. I have fought hard for processes based on theory, and have since realized the mistakes I personally have made in My career. It has certainly been an eye opener.

I am creating a small website. I am looking for help from a friend. That friend wants a business plan. That business plan will be over 10 pages in length when finished. This is for a project that will ultimately be a tiny fraction smaller then the project undertaken by the Drupal community when they quickly implemented this review process. This was a huge mistake. Again, I have made these mistakes in the past, and this is a learning moment for Me, and should be for allot of people.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Quality assurance...

Ryan Weal's picture

"Good question! Since we have a Review process and not a Quality Assessment team..."

See: http://groups.drupal.org/quality-assurance

I'm not even sure what this is about...

Ryan Weal's picture

"I am creating a small website. I am looking for help from a friend. That friend wants a business plan. That business plan will be over 10 pages in length when finished. This is for a project that will ultimately be a tiny fraction smaller then the project undertaken by the Drupal community when they quickly implemented this review process. This was a huge mistake. Again, I have made these mistakes in the past, and this is a learning moment for Me, and should be for allot of people."

What?

Theory and data

sreynen's picture

I set the scope of "Broken" to presenting an error message that prevents execution of features.

Seems reasonable. So of the ~8,000 modules, how many did you test for error messages? And how many had them? And how much of the brokenness is from maintainers who went through the review process vs. those who didn't?

D7 seems to be infinite [...] I suspect [...]

We shouldn't be changing processes based on theory when we have actual data available.

Not being able to say X number of modules are in Y state prevents us (and ME) from fully understanding if a process of quality improvement is successfull or a failure.

Indeed. That's exactly why I'm skeptical of your claims that the review process is a failure.

All of the questions I asked could be answered with public data on Drupal.org. For example, every project has an "issue statistics" link that shows how many issues are in each state and average wait times for various types. Add those up, divide by the number of modules, and you have a pretty good overview of some general trends. And every module has version numbers to give us at least the maintainer's opinion of what state the module is in, and the existence or non-existence of a D7 release.

That data will take time to collect, of course, but that doesn't seem like too much to ask of someone proposing significant changes to how our community operates.

Data

MGParisi's picture

4 Months since d7
- Oldest Review was 4 months old
- Longest response wait: unkown
- Number of Review open issues: 247
- Full Projects - Was not Recorded

8 Months since d7
- Oldest Review: 24 weeks 7 days!
- Longest response wait: 10 weeks 3 days
- Number of Review open issues: 384
- Full Projects - 2214 D7, 5997 d6

Unknowns
- Sandbox Modules - Data not available (can not search sandbox items by Drupal version)
- Number of D6 Modules at 4 Months
- Number of D6 Modules at 8 Months
- Quality of Modules D6 Modules at 4 Months after release
- Quality of Modules D6 Modules at 8 Months after release
- Quality of Modules D7 Modules at 4 Months after release
- Quality of Modules D7 Modules at 8 Months after release

References'
This post
http://drupal.org/project/issues/projectapplications?order=created&sort=...
http://drupal.org/project/modules?filters=drupal_core%3A87%20bs_project_...


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

I took some time and had a

patrickd's picture

I took some time and had a sample look at 100 randomly chosen full projects (I know 100 are not enough to make a reliable statistic):

21 were fully maintained and released for drupal 7.

12 were not ported to drupal 6 (also no port planned).
42 were not ported to drupal 7 (also no port planned).
6 are planned to be ported to drupal 7.
46 were abandoned / not maintained for more then one year.
5 were seeking for a new maintainer.
11 were never released (for more then one year).
10 were never leaving dev (for more then one year).
9 were still under development.

(one module can have multiple attributes)

Actually, a random sample of

dsnopek's picture

Actually, a random sample of 100 from 8,000 (1.25%) would be statistically valid (in the US we do political polls on a smaller percentage) if the sample was truly random.

So, the most important question would be how did you select these 100? Does your random number generator (if you used one) have sufficient entropy to be truly random?

Anyway, I'd be interested in the "abandoned / not maintained". Did they get any traction before going into this state? Or were they created, never really used by many people besides the author and then abandoned? This could be gauged with the number of reported installed (or historical data of them). I suspect that a lot of the abandoned modules could simply be "archived" and hidden from view because no one really used them anyway. But that's just a guess.

My experience with the module ecosystem in Drupal has been 90% positive, but I tend to only use modules that are widely used. And I guess in a way, I'm part of the "problem" because I haven't updated any of my modules to Drupal 7 yet either, but that's because I haven't started using it yet. ;-)

Best regards,
David.

To be honest, that just was a

patrickd's picture

To be honest, that just was a quick shot and only took me one hour. I'm shure it can be done better by spending more time on it.

I simply went to http://drupal.org/project/modules which gives me 341 pages of modules (sorted by title). 341 / 100 ~ 3 pages .. So I picked the first module of every thid page (rather systematic than random, but it should do its job for a kind of qick statistic)

Yes, most of these abandoned / not maintained modules had a low count of users, were in alpha/beta-state and weren't maintained for a year.
Really pity: Most of the active and maintained were drupal 6 modules (really interesting ones I really would like to see in drupal 7).. these ones didn't even have a 7.0-dev :(

regards

Formal verse Non Formal Review Processes

MGParisi's picture

A person who is working on Drupal, comes in and installs the latest version (D7), then looks for a module http://drupal.org/project/imagefield_archive They see that only 31 Sites use it. They then look into the issue queue and finds no requests for it to be ported to D7. They Decide to re-write the module from scratch instead of porting an existing module. They wont port it because it seems to be more work then worth it. This is not a fictional story, it played out just like this last night. The person doing this is a veteran of Drupal and has many more years then Me (They where around when D5 was the only version).

Now this person could post their module without a review because they have access to GIT, and have enough Karma too get away with large scale mammal slaughtering. I am not critical of this user, in fact I support their decision to do what they did as I would of done the something. In addition a D5 Amazon module I inherited was succeeded with a D6 version in an identical way! This seems to be a common story for smaller lower maintained modules.

Review Process

1) Module is Added to Sandbox
2) Module owner files a request for GIT access (starting the Review Process)
3) A volunteer assess the module and approves the module (Goto Step 5), or makes suggestions and improvements (Goto Step 4)
4) Module owner fixes module and updates request for GIT access. (Goto Step 3)
5) Module becomes a "full project"

Natural Open Source Review Process

1) Module is added to the module page
2) Module consumer look at the module and issue queue and gathers information regarding the module
3) The Module Works as intended (Process Ends), The module fails to meet expectations (goto step 4)
4) Module consumer decide to search for another module (goto step 2), develops a new module (goto step 1), Files an issue, Patch or feature request, improvement, or writes documentation.
5) Module owner Applies Patch, Makes improvements, or fails to respond.

Review Process Conclusions

The review process is that it takes upto 6 months to release a module from sandbox to module status. Its difficult to finding any sandbox modules (example: when you attempt to search d7 sandbox modules you get 0 results). Installing a sandbox module can require knowledge of GIT which further limits its use. The formal review process is reliant on volunteers. Thus Sand boxing a project limits its exposure to module consumers which lowers the numbers of module consumers and restricts the natural review process.

We CAN conclude that the reviewed projects will certainly be a higher quality then non-reviewed projects. We can conclude that the review process can take up words of 6 months.

Natural Open Source Review Process Conclusions

The Natural Open Source Review Process takes place every time a consumer looks at a module page. The natural review process makes every Drupaler a Module Reviewer. The Consumer is the quality assurance specialist.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

Difference between Quality and Review

MGParisi's picture

A Quality process is an on going process. It would work to achieve an atmosphere where module developers get feedback from module consumers, and module consumers get the best information available for choosing a module. A Review process is a period of time when one item is reviewed by an individual or team. The review process has a start, begining and an end. Likewise a Quality process never ends, its on going.

Instead of reviewing module developers, I purpose a two stage process. First lets get people access to GIT, and publish modules too the normal module area. Second lets monitor modules by utilizing our greatest asset, the consumers of the modules. Consumers will also serve to educate the module developers when issues are filed or with patches that fix failures.

Lets spend our volunteer's resources on ALL modules instead of new module developers. We can encourage module owners to include images, better descriptions, documentation and readme's to provide consumers with the best information to make the wisest choice. A module that is not maintained and that is reported as broken, should be addressed as such and warn users of its current state. Abandoned projects should be assigned as such, and we should listen to the consumer of the module to be the expert of what "Quality" is.

The security team can not verify the security of all 8,000 modules. We can however provide reporting features for users of modules to report troubled module's and to provide a mechanism to get changes implemented, to get new (co)maintainers, and to allow for more growth.

Looking at all of the posts, all of the issues, all of the statistics, and calculating in the support requests, improving Documentation and promoting module maintenance seems to be a no brainier. We have a finite set of resources so for me it is clear where those resources should be spent.


--Sig--
Owner of Proper Programming, LLC a software and website development firm.

We have both

sreynen's picture

I don't see anything in your proposal we don't already do on Drupal.org. Everyone already has access to Git, so there's step one. And we can already see what "consumers" think about modules via both usage statistics (they apparently like it enough to keep using it) and issue queues (they have enough problems to open issues), so there's step two.

For those of us who use sandbox issue queues to do reviews, the feedback process is identical between sandbox projects and full projects. There's nothing about reviewing sandbox projects that prevents a volunteer from giving feedback full projects. I do both, and I expect most reviewers do both, so we already spend our volunteer resources on all modules.

If the problem you're trying to solve is not enough feedback on full projects, I'd suggest this is the wrong place to solve it. Beyond being almost completely unrelated, it's also a completely different scale. We don't even have enough people here to clear out a queue of less than 100 projects awaiting review and you're talking about 8,000.

I think pushing abandoned /

patrickd's picture

I think pushing abandoned / not maintained full projects back to sandbox status automatically would already help us.
This would also free up nice namespaces; space for new, maintained projects (not everyone wants to takeover the project of someone else)

eg. project names like "debug" http://drupal.org/project/debug are may very interesting for a new kind of module

Kicking projects like this back to sandbox (within the possibility of preventing this in a period of time) would give us at least a better overview.

My two cents

ccardea's picture

I see this is once again a hot topic on Drupal.org. In recent months I've been personally involved on both sides of the review process. A couple of weeks ago I went through and analyzed this issue. This is the conclusion I reached.

Because of licensing issues and security concerns, it seems like some form of code review is necessary on Drupal.org. But because of the high volume of submissions and the lack of available manpower, code reviews need to be kept to a bare minimum, which would include licensing, obvious security vulnerabilities, and minimal coding standards focusing on possible conflicts with core or other contributed modules. In particular, no attempt should be made to enforce any kind of duplication standard, or to do a detailed technical review. All code on Drupal.org is open source, and it is specifically stated that users are granted a license to use and modify the code. It is not up to Drupal.org to enforce the copyrights of existing contributors, and if there is a complaint, offending projects can be unpublished...

If you want to read the whole thing, it's at http://ccardea.com/node/13. I should add that all GPL licensed code is distributed without any warranties of any kind, and it seems futile to try and enforce a quality standard, unless you're trying to build a brand. I don't think you can or should try to build a brand on the backs of volunteers.

I looked around on the internet to see what other sites are doing. Customer rating systems seem to be what works on the larger sites. They create a sort of natural selection process in which the most popular modules rise to the top and it's difficult for a newcomer to get any visibility. Of the very few sites I found that attempt to do some sort of code review, the volume of submissions is much smaller and there is a company in place with people available to do the work. Sites like PECL and PEAR have some fairly strict standards, but that's really a whole different ball game than Drupal.

I don't think the module approval process alone will kill Drupal, but it will contribute to its decline. It is a symptom of an organization that has some problems.

on spot

eigentor's picture

I think you have a very good point here.
Try to implement a model of incentives - trying to get good ratings - instead of a system of resctrictions - people having to follow coding standards.

It is an interesting way of reversing the order altogether. People searching for a fivestar module and finding 25 instead of 5 remains an issue nonetheless. But this comes back to making it visually very clear which one is more popular and maybe better. That's the way you do it on Android market - which by the way is a much worse mess then d.o. is :). Well the android market is a terrible example -as a whole it is rather the opposite of what we should do - but people will find better ones.

Life is a journey, not a destination

Freedom to contribute

DevElCuy's picture

We have not lost our freedom to contribute, just need to find the right model to motivate and facilitate contribution. Current model makes motivated people wait for half a year to get their project approved, we need to fix that. Just my 2c.

--
[develCuy](http://steemit.com/@develcuy) on steemit

People have the freedom to

greggles's picture

People have the freedom to contribute (via git sandbox), but not the freedom to do whatever they want.

Also, the half year cases are, in my experience, cases where the user skipped some of the important steps they could have moved through on their own (e.g. code style, duplication).

Is is possible?

Diogenes's picture

Is it possible to bump a discussion this old? It has been over a year. It was a recent discovery for me.

The review process IS killing Drupal. Waiting for a review has gone from a wait time of 5 days to 5 weeks to 3 months.

Meanwhile, nothing has been done. Why is this no surprise? The answer is very simple.

This is just one of the reasons I'm looking at other options.

This Process Is Broken

People can get approved now

klausi's picture

People can get approved now within 2 weeks. Just take part in the review bonus program: http://drupal.org/node/1410826

Any reasons for not applying

Diogenes's picture

Any reasons for not applying this process to everyone? Why should it be only the new members waiting for review that are the only ones who have any skin in the game?

Other view - Sandbox Modules Packaging

Anybody's picture

Would it make sense perhaps to add packaging to sandbox projects so that sandbox projects can be used more easily and it's not so bad anymore to wait for approval?

No, because sandboxes are

klausi's picture

No, because sandboxes are defined explicitly as experimental code. They are not meant for blind use in production.

Hi Klausi, please don't get

Anybody's picture

Hi Klausi,

please don't get me wrong. Of course you are right. It should no way look as if such a sandbox would provide something stable.

My idea was more like on Github Projects to provide a download button for easier first "testrun" of the module.
Github doesn't care if the project is a sandbox, it's the users and developers job to evaluate the code quality.
Furthermore I am sure that dedicated developers will always finally want to create a clean full project even with that option.
It might just speed up and more-open development a bit.

In recent month I've seen lots of really good sandbox modules that were not ready for production or still awaiting approval. But they were still a good starting point to test and decide then if it is worth to contact the module developer, create custom solutions or others.

It's still much more work to check out the sandbox code, if you don't have the required development environment. Even if you have, a "one click solution" may be interesting.

But finally as I said I understand your point of view and my points might be wrong or dangerous perhaps. It was just important to me that we could perhaps find further ways to solve or reduce the problem and stay open minded.

I got put off when I spent

zenlan's picture

I got put off when I spent about 7 hours reviewing a very complex module that had lingered at the back of the queue for nearly 6 months and got 1 review bonus point while someone else also got 1 point for about 20 minutes reviewing a tiny, new module. The old, complex module made it through the process and I helped another one through but I gave up.

All in all I spent about 27 hours on reviews in a 3 week period, earning about 6 or 7 bonus points iir. The concept of the process is good but in practice it seemed a bit unfair. I guess I was unlucky to do it at a time when the queue had nothing but big, old modules in it.

A code review is hard work

Diogenes's picture

+1 -- A good code review is hard work (unless you're one of the old foxes like Klausi who can just look at something and provide great advice - I digress).

You can now assume that if a module passes coder and pareview without anything serious issues, the module is ready for a functional test, which means proper context.

Anyone who has had a bad code review knows that getting reviewed by a newbie who is trying to score points can be a very bad idea.

UPDATE: I stutter when I get nervous

Just A Few Good Men?

Diogenes's picture

OK - I'll be the first to admit that the first blog was a little over the top. Here is more rational rant...

http://sontag.dev/blogs/just-few-good-men

After 2 years, a little over 37% of new contributions have cleared the wall. I reckon a 55% attrition rate. What is this? A university degree from Oxford?

Meanwhile, there is all kinds of flotsam and jetsam floating around on the Drupal.org website.

Linking to your dev hostname

fuzzy76's picture

Linking to your dev hostname might not be helpful for the rest of us, out here on the internet ;)

Doh!

Diogenes's picture

@fuzzy76 - Thanks for that.

Correct link for above

http://sontag.ca/blogs/just-few-good-men

(Curious - For some reason I can't edit the original post)

great start at analysis

greggles's picture

That report is a great start at analysis of the queue. It provides an interesting overview of the states.

The 172 that are duplicates shouldn't be considered part of the group of "not made the cut or given up." In my experience those are cases where the person submitted two applications or got approved in some other manner. The same holds for the 7 "works as designed." issues.

If you want to determine ways to improve the application process I think you need to look into the detail of the groups a bit more (perhaps by doing sampling on anything other than the "fixed" groups and trying to determine a "root cause" of why the application was abandoned). I've seen a lot of these things:

  • Maintainer doesn't bother following the guidance and submits a project with one or more of LICENSE.txt, code that violates our policies on copyright/3rd party code, code that doesn't pass basic formatting checks, code that doesn't work. This is demoralizing for the reviewer because they have to say the same thing over and over in each new application. It is demoralizing for the applicant because they are getting their code style nitpicked (IMO the root problem here is applicants who don't care to follow the community norms and the process should either educate or weed them out - that is by design).
  • Maintainer submits code that relies on one or more third party web services (maybe even requiring a paid subscription) which makes it very difficult to review the code - I don't see a way to really fix this issue.

Your conclusion in that article is that we should stop worrying about duplication, but in my experience very few project applications are stalled out by the duplication check.

What is this? A university

duckzland's picture

What is this? A university degree from Oxford?

Nope just a degree from University of Drupal. LOL

You may be wrong

Diogenes's picture

"in my experience very few project applications are stalled out by the duplication check"

I did not look that hard, only at a few that looked suspicious. Here is one example:

http://drupal.org/node/1541986

You may be wrong about that. It's a quick response (duplicate!) that makes the queue smaller. If the creator has given a reason why it it better than a current one, then let it go. Either that or have a voting system in place with a place for comments, like StackOverflow.

There is a touch of imperialism in the community.

I've done hundreds of reviews

greggles's picture

I've done hundreds of reviews and you think a sample of size 1 should compel me to adjust my thinking?

That issue in fact was not a "quick response" - patrickd gave a lot of advice about issues with the module and project page.

In this case your are guessing about what happened to mishac - there are a lot of possibilities about why they decided to abandon the process and your theory is only one of them.

I've been watching the

ELC's picture

I've been watching the comments on this for a few days and decided to read the comments on the Post Bookmarklet project application to make my own determination on the project. I used to review quite a few projects, but unfortunately, $dayjob has taken over my time but I think this is certainly important enough to take time off work to reply to.

I take particular issue with the comments of Diogenes on that application which I find rude and counter productive to building a better community, or getting that project moving forward.

patrickd's comment was needed, and was not offensive. If there is an old module that is abandoned, as the author admitted, and it just needs to be updated quickly with the code that is already written, then that is the path to follow instead of a brand new project. It is trivial to download and branch the existing code to a new version, make the required changes including making a high quality addition and following the coding standards, and then submit to take over the abandoned project. The newly completed code even completely skips the "git vetted status" project application queue.

We already have a problem on the d.o site with projects being lost in the forest, or simply not being up to scratch. (If you find such a project, please submit patches or take over maintenance.) Opening flood gates because some of the full projects have issues is not the answer. If even a small percentage of the projects submitted to the queue were allowed straight through the overall quality of projects on the d.o site would quickly fall to zero. I have only seen 1 project that was up to scratch at the time it was submitted and as a result, it made it through the queue in under 2 days.

That is the biggest issue that I have with people complaining about the length of time it takes to get through the queue. I took the time to learn what was required of a project before even contemplating joining it. I read what other problems people were running into, and what the common obstacles were. Only after learning that, and making my application confirm to those norms, did I submit an application to the queue. It took less than 10 days to get through as I still had some things that needed to be fixed.

If everyone took the time to prepare themselves and their project for the queue, then it wouldn't be so painful to get through for either the submitter or the reviewers. And yes, it is painful for the reviewers. Downloading yet another project that is so badly put together that it needs another 40 hours of reviewing to get it to an acceptable standard is incredibly demoralising, especially when the submitter has not even read a single scrap of associated documentation related to submitting themselves to the project application queue.

Without rules, there is chaos. We don't let everyone drive on our roads; We don't let everyone have a gun license; We don't let everyone fly planes; We don't let everyone create random projects on the d.o website. Instead, we expect all of those people to take the time and effort to learn what is expected of them to continue the forward progress of an evolving, working system.

Slow down

Diogenes's picture

A little context here. The first line in my comment was "Hear Hear!"

It was a way to upvote a previous comment which I shall quote here:

"There is no good bookmarklet for Drupal at this time. I really don't see the need to discourage this guy...
Please go on mishac, and do a new and good module if you want to."

patrickd did raise some good points, I don't dispute that. Maybe he was being a little too thorough. You don't raise a duplication issues this late in the process.

Of course, if you can point to a page anywhere that deals with how to deal with abandoned projects that should have been cleaned out of the drupal.org barn long ago, but still exist, please correct me.

You don't raise a duplication

greggles's picture

You don't raise a duplication issues this late in the process.

The very first thing patrick said in that review was "welcome."

The very second thing patrick said was a question about duplication. What do you mean that he should have raised the issue earlier?

I'm not a big fan of lists.

Diogenes's picture

I'm not a big fan of lists. I read Stephen Covey's '7 Habits' book and thought it was pretty good. Then somebody asked me what the best habit was. I did not have a good answer; had to think about that.

I'm not biblical or even remotely religious. Covey's best habit was...

"Seek to understand, and then to be understood."

You speak from your experience, I speak from mine. There is more than one example here.

As long as we're sharing life

greggles's picture

As long as we're sharing life philosophies: I'm a big fan of data.

Your claim that there is more than one example here is exactly right. Please do dig into more than one example and come up with a root cause analysis (like the folks in London did).

One fact that I found interesting from the Munich review: 111 security issues were found and tagged in the 1,742 applications. I imagine that many more than the 111 were found and fixed as a side-effect of a coding standard cleanup or the code style requirements that are run before someone even applies. Consider that in the context of security advisories released per year. In 2012 we have just over 150 security advisories. In 2011 we had 59 of them. Consider, also, the education effect. Most new applicants go on to make more modules but they bring along the learnings from the applicatino process. So, the review process has greatly reduced the number of insecure Drupal modules in the world.

What's wrong with taking over abandoned projects?

sreynen's picture

I've suggested many times that a new contributor should take over an abandoned project with a similar purpose. Sometimes this is met with the enthusiasm I think it deserves, as it's an easy way to get a bunch of users while skipping the review process. But more often, new contributors are opposed to the idea. I don't generally push the issue, as I don't want to make it seem like a hard requirement, but I would like to better understand the perspective that is resistant to taking over an abandoned project. Diogenes, you apparently share this perspective, so maybe you can explain more why taking over an abandoned project seems like something to avoid?

Suggestions to adopt abandoned projects are certainly a small part of the review process, but it looks like the most promising avenue to getting something useful out of this discussion, assuming that's the goal.

A few reasons

fizk's picture

From my own experience, here are a few reasons why I might not want to take over an abandoned project, whether or not it's my first Drupal module:

  1. Don't like how its written
  2. Don't want to have to learn someone else's code, esp. if it's extensive
  3. Don't want to be associated with the failure of that project
  4. Don't want to support existing users, even if I continue development on a new branch
  5. Don't want to create a migration path from the old code to the new code

And when we recommend that a developer contribute to an existing, actively maintained project, one of the reasons why they might not want to do this is:

  • Don't want to play second fiddle - rather have complete control over everything (project page, codebase, releases, direction, etc).

I'm just being honest here, but I think some developers, including me, get a big kick out of owning their own Drupal module. Taking over another module, or having to share control with other developers, just doesn't quite cut it.

Thanks

sreynen's picture

That all makes sense, thanks. Most of that is completely optional when taking over an abandoned project, and maybe we should make this clearer when suggesting the abandoned project path.

Don't like how its

greggles's picture
  1. Don't like how its written
  2. Don't want to have to learn someone else's code, esp. if it's extensive
  3. Don't want to be associated with the failure of that project
  4. Don't want to support existing users, even if I continue development on a new branch
  5. Don't want to create a migration path from the old code to the new code

1 and 2 are easily solved: it is not a requirement that you take over their code. Create a new branch and put in 100% new "good" code.

3 seems like a marketing issue. Again, new branch "new maintainership" mention on the project page. I've heard the phrase "X module sucked but the new branch is awesome" so many times. I think people have a lot more respect for that (I know I do).

4 Mention that you won't support them on the project page. Simple enough.
5 That's not a requirement. Someone else can write it.

These seem like common misconceptions about taking over a project and maybe we should document them as such and include it on the "taking over an abandoned module" page so people won't be worried about them.

sreynen,greggles, I agree,

fizk's picture

sreynen,greggles,

I agree, it'd be great to explicitly mention that 1,2,4,5 are optional

A) when asking someone to take over an abandoned project
B) on the "taking over an abandoned module" page

because it's not immediately obvious that they are optional.

If 1,2,4,5 are optional though, taking over an abandoned project simply means using the same project name and git repository. If the abandoned project doesn't have many users already, the developer is stuck using a particular project name and git repository for no obvious benefit.

I tried to state them as

greggles's picture

I tried to state them as positively as possible http://drupal.org/node/251466

Thanks! That looks great.

fizk's picture

Thanks! That looks great.

Thank you greggles et. al.

Diogenes's picture

Thank you greggles et. al. for the efforts in improving the documentation on taking over an existing project.

My own situation was a little odd. I decided to change the name of my project. Drupal.org was down at the time. I googled for different names and Google had a cached page for the 'Members' project. I saw it was abandoned a long time ago, so I choose that. Changed the function names on 3 different versions I had committed (6,7 & 8).

When Drupal.org was up and running again, I tried to change the name to Members, but it was not allowed, so I selected 'Members Page' instead.

So the project is Members Page, but the function names are prefixed with 'members_'. Everything works perfectly fine, except drush.

There is something about this that a drush install/enable command does not like (maybe an option missing?). Two people reviewing my module have complained about this although a conventional install/enable works.

I have been charged with writing crazy code and not testing properly. I have done some pretty rigourous testing. It has been over a year, after all, since I first started this.

It was this naming conflict that triggered a suggestion for me to take over an existing module. Grrrrrrrr....

It has been suggested by some else that abandoned and obsolete projects be sent back to a sandbox state, and the namespace be made available again.

I think this is an excellent idea. Drupal.org really does need to have it's closets cleaned out. There is so much cruft in there now.

It was this naming conflict

greggles's picture

It was this naming conflict that triggered a suggestion for me to take over an existing module. Grrrrrrrr....

I don't see why that was so bad. As far as I can tell you haven't taken any steps towards that process: search. So...give it a shot. I imagine you'll find it quite easy.

Drupal.org really does need to have it's closets cleaned out. There is so much cruft in there now.

Your sentence lacks a sense of who will be doing this work. There's only one way to guarantee it will get done and that is for you to do it. It's not easy, but there is a whole section for how to make drupal.org awesome. Please stick with it for all of our betterment!

Thank you for the link

Diogenes's picture

"I don't see why that was so bad. As far as I can tell you haven't taken any steps towards that process: search. So...give it a shot. I imagine you'll find it quite easy."

Thank you for the link to the issue queue to a project that is only available in versions 4.7 & 5; where the last commit was over 4 years ago; and for a module that is only used by 50 sites. I'll be sure to follow those 4 basic steps you mentioned, wait two weeks, and then apply for permission to take over the project.

But my first priority is to convince w3.org that the world really does need a <sarcasm> tag.

In the meantime, try to imagine how much I care.

"Your sentence lacks a sense of who will be doing this work. There's only one way to guarantee it will get done and that is for you to do it. It's not easy, but there is a whole section for how to make drupal.org awesome. Please stick with it for all of our betterment!"

And thank you for being patronizing. It proves earlier points.

I did not write the original post of this discussion. I just agreed with it and bumped it. It has 18 votes now, which I'll wager is more votes than any other post you have made greggles.

I also fully support what mlncn had to say at Drupalcon Munich. The double standard of subjecting only new members to code review is untenable.

So let's get back to the real subject at hand -- Module Approval Process will KILL Drupal. We've beat the duplication issue to death.

The community's processes

greggles's picture

The community's processes have grown up over time and I think the 2 week waiting period makes sense. If you think a policy is inappropriate you can certainly work to change it, but you'll have more success with if you work on them in the appropriate channels (not here) and use a more positive tone. And while you may say you don't care I think you really do. You've done some solid research and posted in multiple places about some problems you've identified. I hope you continue to care - the Drupal community is nothing if not caring and passionate.

Sorry if that came across as patronizing, it certainly wasn't my intention. My intention was to inspire you that if you want to see change it is absolutely possible but going around complaining, without facts, and without a willingness to do the work towards the fixes is not a way to rapidly make change happen. There are problems here and they need more people to work on them. I hope you'll consider doing that.

Footnote: I just checked and I actually have 1 post that's higher rated. Besides, voting up on a post is easy. Doing work to fix the problems is hard and I've done a lot of that. So, let's all do more of that and less vote checking, alright?

Check this out:

Sylvain Lecoy's picture

Check this out: http://drupal.org/node/1351312

Took me 7 months to get approved, although I had a great project page, documentation written, a high percentage of commentary by LoCs, a demo site, and even some automated tests. Once I started to use the review bonus system it went more quickly.

Wow 7 Months? you should be

duckzland's picture

Wow 7 Months? you should be grateful man, That was FAST!. mine took 1+ year. LOL

Actually it took you and us a

klausi's picture

Actually it took you and us a year to follow-up with the application in time. So yes, reviewers and applicants can be slow with their response times.

yeah it is very slow, but

duckzland's picture

yeah it is very slow, but thanks man for your review. not too many people willing to do reviews.

What are your thoughts?

Diogenes's picture

I did check that out link. Had a few chuckles too (the idea of reviewing your own module under a different name). You had some great reviews. klausi is awesome. jthorson is very good too, probably because he went through the process himself and is very empathetic with those that are in it. Though it did take a long time, I sense you learned a lot, like I did, and as a result, you are quite capable of giving a good review yourself (I read those too).

I also thought you may some good points too in your 'Golden Hammer' post on Views. I spent a whole day learning and experimenting with Views before I realized it could not do aggregation (I know, now it can).

Then I spent just 1/2 a day writing my own module that did. That was one of the first modules I ever did. Would I replace this module with a Views equivalent now that views can do aggregation? Not likely.

I think the best way to solve this issue is to make code reviews part of the Drupal ethos - we are all expected to do code reviews and we all expect to be reviewed.

It would improve the quality of the existing contributed modules and solve the problem of the lack of people doing reviews.

What do you think Sylvain?

I personally think we should

Sylvain Lecoy's picture

I personally think we should have unit of measurement for code quality, something which is as physical as the 'Automated Test' tab.

I know that Sonar can pass a set of defined rules to check the project compliance. Adding it under a 'Quality Assurance' regrouped together with the test coverage and so on will put everyone on the same level. Sonar can work with Drupal Code Sniffer as well. Here is their demo web site: http://nemo.sonarsource.org/.

That was for the infrastructure thought.

Now from a philosophical point of view, I think having this indicator will help people realize what is code quality and respect to standard. PAReview is already doing this kind of stuff, thinking of integrating it in every project pages (even project/drupal core itself) and adding an indicator of the project code quality would help people realize that they are still under review process and that they should lead by example because their projects will teach newcomers. This would also make code reviews part of the Drupal ethos.

We could also set a system of badges and point like stackoverflow to encourage people do reviews... but that's another story..!

Hear, hear for StackOverflow

Diogenes's picture

I think the StackOverflow model would be great for Drupal, for support, for reviews, for debates and for reputation building.

There is a Drupal StackExchange site. Now go to drupal.org and try to find a link to it anywhere on that site. I'm pretty disgusted by this.

Have a look at this discussion here...

drupal.org/node/1236290

All talk, no action. More people have to ask why we have to keep eating our own dog food. The learning curve for Drupal is still too steep. This is one of the reasons why.

The next generation testbot

jthorson's picture

The next generation testbot initiative is repositioning qa.d.o as more of a generic job scheduler and automation platform. I have already completed initial plugins for the Drupal Codesniffer and PAReview.sh scripts on this new platform, and can demo the qa.d.o/testbot pieces for these components.

However, these are blocked on the drupal.org migration to D7, as well as development of the actual user interface for the 'quality assurance' tab (to replace today's 'automated testing' tab).

So yes, things are moving in the right dirction ... And we should start seeing some significant improvements hitting deployment around the DrupalCon Portland timeframe.

Let's work on solutions, wiki-style

sreynen's picture

This discussion is pretty heavy on describing problems (which I think everyone agrees are problems) and light on working on practical solutions (which is hard, because this is all complicated). I was going to leave a comment to try to change direction more toward solutions, but realized that this is a wiki, so I can just edit the initial post, which I think is what set this off in a less productive direction.

So I've edited the document to focus on finding solutions. This meant deleting large portions of what was there, which was full of first-person pronouns that don't really make sense for a collaborative wiki, and also a lot of out-of-date information. The revision history is still available, if anyone wants to go back and see where this started.

But going forward, I suggest we move past the rehashing of the problems (though if there's something I missed in the wiki, definitely edit it) and move into figuring out solutions, including difficulties with those solutions (e.g. new problems, no one to implement, etc.) that have prevented us from enacting them already.

We all have different ideas, but everyone here is interested in improving this, so let's get to it.

I think it rather unfortunate

Diogenes's picture

I think it rather unfortunate that the original post was turned into a wiki page, because now it has morphed into something that has lost it's original impact and intent.

The original title -- "Module Process will KILL Drupal" was intended to capture the attention and set off alarms. Though it may have been filled with "hyperbolic" claims, it did a pretty good job expressing the frustration of some. Have no doubt -- this process is discouraging new contributions and driving potential new members to other Open Source projects.

Now that the post has been stripped of all the spit and vinegar and the title changed to "Module Approval Process is Too Slow", I suspect it will soon become another long forgotten discussion that is all talk and no action, like this one...

http://drupal.org/node/1236290

Sorry, but I won't be contributing to this wiki page. I just don't see much point. There has got to be a better way.

Oops, editing enabled

sreynen's picture

I didn't realize until looking at the revision history just now (to confirm this has always been a wiki) that the input format was set to an admin-only format to combat spam. I just changed it back, so everyone can edit it now. Hopefully spammers will stay away.

Don't go away mad, just go away.

Diogenes's picture

I have had 7 reviews for my sandbox project. The longest I had to wait is 66 days. The shortest time between a request and a review was 47 minutes. And there was one day where there were 2 reviews in 2 hours.

Nobody mentioned that the module had to install with drush, but apparently this is one of the rules.

This process sucks. So I don't care about becoming a Drupal contributor anymore, but I decided I'm not going to just walk away without saying anything...

http://sontag.ca/blog-tags/drupal-diaries

Don't go away mad, just go away.

cubeinspire's picture

Hi Diogenes,

I understand it can be frustrating to stay so long time on the sandbox queue... if you still want to release this nice code I can follow your module to provide a fast review.

Today I installed your module and in my humble opinion I think the blockage is in some sort justified because the module uses curl functions without a real need.

It's important to notice that not all servers have the curl module on their php installation and that the use of this library is producing also an error with drush (removing curl will allow the use of drush).

There is no need to use the curl function because there are Drupal functions that can replace curl function on this particular case as it has been pointed during reviews.

I don't think it takes more than 2 - 3h to replace those 10 lines of code and that can really improve the usability of your code, after all the objective of releasing a module at drupal.org is that it can be used by everyone...

cube inspire - web design and web development solutions !

Thx for the review and offer

Diogenes's picture

Thank you for the review logicdesign, and the offer to fast track (still not sure about the title of the post ;-). Fabianx also posted a review which is oddly encouraging.

I will reply soon; distracted by other things right now.

Promoted Sandbox != Permission to create new Projects

Fabianx's picture

Promoted Sandbox != Permission to create new Projects (Project Nodes)

That might come as a surprise to some, but maybe we should distinguish between the promotion of a module and a promotion to being permissioned to create new modules, which is obviously giving more rights.

Some people in the review queue might just want to have their sandbox promoted to full status and being mentored a little. (and that is similar to if you become a co-maintainer)

That might need a less rigid review than giving the right to create projects at will.

I am not sure if that would complicate or simplify the process, but I wanted to point this distinction out.

Maybe just maybe it would feel "easier" to sponsor a project then to sponsor a "person" and trusting for all time in the future.

Note: This changed in the switch from CVS to GIT. Before having co-maintainership meant always also full project creation rights, now this is no longer the case.

Best,

Fabian

Solution or not ?

cubeinspire's picture

Hi Fabianx,

What you are saying has a lot of sense but I'm not sure if taking this kind of decision will improve the approval speed.

maybe other kind of review that doesn't give rights to create full projects after approval could be less strict ?

In any case its very important to the Drupal reputation to have a security check up of the code and that means to have two-three guys verifying each line of code... I was imagining a kind of automatic system, like the review bonus but where the contributors have to install a module for reviewing ( a mix between coder and a review client ) and 3 - 5 other sandbox projects then send the review automatically... but then we could pass around the security problems.

cube inspire - web design and web development solutions !

Drupbot ?

cubeinspire's picture

It wouldn't be nice to have a bot that welcomes the new sandbox projects and that gives all usefull links and checks the following automatically:

1- Existence of README.txt / LICENCE.txt
2- PAReview.sh code style / warnings / errors
3- Git branch naming
4- Length of the code superior to 120 lines and 5 functions
5- Basic testing (installing, uninstalling, hooks etc), report of fatal errors and warnings.

When all those tests are ok, then the bot marks the project as Ready To Be Reviewed, then there is a human verification regarding API/Seucrity then if its ok its tagged as RTBC then released (or directly released).

I saw that most of the time project applications have those basic problems and that slow down the process as there are not enough reviewers... The bot would allow the applicants to ask an instantaneous test and get tagged the same day as RTBR, and reviewers could then focus on ready projects.

cube inspire - web design and web development solutions !

keep high level of quality thru automatic prereview

chipway's picture

I think review must keep high level of quality as a target.

@logicdesign,

Great idea.

This would save time both to contributors and reviewers. As reviewing is time consuming, we could focus .

I aggree too that we find these mistakes often. On my own I begin every review with PAReview. If I get many errors, I am not motivated to go further.

Ready To Be Reviewed (RTBR) could be confusing with RTBC. What do you think about "Needs Project Automatic Prereview" (NPAP)?

In fact, do we need this step? May be we should require that every projet Application comes after this automatic testing run by the applicant.

How could we help automating this part of the process?

Of course, after automatic preprocessing, manual review is mandatory.

Review Sprints

chipway's picture

It helps new contributors to raise their expertise too.

We should try to increase the number of reviewers. Organizing "review sprints" could help.

It could also be part of the Drupal Ladder initiative.

Still very broken

fuzzy76's picture

Just look at applications like this one. Every single time the applicant satisfied a review, ANOTHER reviewer stepped in with a completely new set of issues. This ping-pong has been going on through EIGHT distinct reviewers and 17 months. I can't believe people actually get approved through this kind of process. And I can't believe the guy haven't packed up and moved on to more open communities.

Is there any reason that some modules have to pass a comittee of eight reviewers when others merely needs to satisfy a single reviewer?

Review bonus scheme is bloating things

Frank Ralf's picture

IMHO it's the review bonus scheme which is hampering progress and bloating application issue queues at the same time ([META] Review bonus).

As every applicant is required to do "at least 3 manual reviews of other applications" everybody is eagerly looking for projects to review to promote their own project, whether they are interested in the reviewed projects or not.

Just my 2 cents.

EDIT:
What about something like the Firefox approval process which distinguishes between a Full Review and a Preliminary Review:

  • Full Review — a thorough functional and code review of the add-on, appropriate for add-ons ready for distribution to the masses. All site features are available to these add-ons.
  • Preliminary Review — a faster review intended for experimental add-ons. Preliminary reviews do not check for functionality or full policy compliance, but the reviewed add-ons have install button cautions and some feature limitations.

    https://addons.mozilla.org/en-US/developers/docs/policies/reviews

I don't understand your

greggles's picture

I don't understand your criticism of the review bonus. I can see how it's causing a lot of comments (reviews) but I don't see why the review bonus would bloat (i.e. cause there to be a lot of open issues).

I do see how the bonus process would lead to reviews that are mixed quality, but I'm not sure what more to do about that. There is great documentation on how to perform reviews and we can only ask people to try their best.

The idea of two different kinds of reviews is interesting. I wonder if people who are git administrators should say "this is a full review by a git administrator" and we link that to something that explains what it means - i.e. that the words are carefully chosen and should be heeded carefully.

It's easy: more reviews are

fuzzy76's picture

It's easy: After an applicant has fixed his/her project after a review, more reviews are piling up on the project before the original reviewer manages to set the application to RTBC. Which means the project will never be good enough, because someone will always find something more to pick on. This causes the actual standard for getting projects approved to become impossibly high.

Make sandbox projects more prominent

Frank Ralf's picture

Just some more thoughts on these issues:

Finding a sandbox module

To engage more people to review other peoples code this code has to be findable in the first place. As it happens to find a sandbox module which provides a certain feature (e.g. "ownership") you have to follow at least these steps:

  1. Search on drupal.org: https://drupal.org/search/site/ownership
  2. Filter by module: https://drupal.org/search/site/ownership?f[0]=ss_meta_type%3Amodule
  3. Set status to also include sandbox modules: https://drupal.org/search/site/ownership?f[0]=bs_project_sandbox%3A[%20TO%20]&f[1]=ss_meta_type%3Amodule
  4. Only then will you find Delayed Ownership (which happens to be my current project in the application issue queue).

So IMO it's no wonder that it's only other applicants which stumble across sandbox modules and review them. Another way to find sandbox modules is http://drupal.org/project/issues/projectapplications and http://ventral.org/paissues but I suppose that these pages are only visited by other applicants.

(NB: No sandbox module is listed on http://drupalmodules.com which IMHO is still far better for finding modules than drupal.org.)

I think the main flaw of the whole review process is the fact that sandbox modules are hidden from the masses. Why not make them more prominent to engage more people in the review process?

Some suggestions:

  1. Make "All projects" the default setting when searching for modules on drupal.org. In fact, a filter should be set by default to provide the largest possible result set and then be used to further narrow your search, not to broaden it.
  2. Mark sandbox modules clearly as "preliminary" in the list of all modules as suggested above.

I don't have anything against nudging people to contribute more to Drupal but the resources should be applied where they are most useful. I contribute to
quite some issues
depending on problems I'm facing in current projects.

Reviews of sandbox projects should preferably done by people which have an intrinsic interest in the sandbox project features not by other applicants only doing the review to earn their bonus points to get their own project promoted.

I hope this clarifies my criticism of the review bonus.

It's interesting to note that

greggles's picture

It's interesting to note that Firefox's add-on listing makes the sandbox items harder to find than we do:

While waiting for review, add-ons will not appear anywhere in the gallery publicly, but direct links to the add-on's details page will work. This allows the author to blog or share the link with friends, inviting them to try it out, even when not yet reviewed.

From an application doc.

I consider reviewing other

klausi's picture

I consider reviewing other people's work an important experience for every contributor. People get approved within a month these days if they are lazy, or within days if they have a bit more time to review other applications. That is all possible with review bonus, and I'm eager to review/approve you if you help me out a little. We are a community and we help each other, and then nobody has to wait.

There are 20+ GIT

g089h515r806's picture

There are 20+ GIT administrators, assumed that each one review 100+ project per year. That meand 2000+ projects will be reviewed.

It is enough for Drupal projects.

I do not know why so many people complain about this process.

我的drupal博客Think in Drupal

There is so much wrong here,

fuzzy76's picture

There is so much wrong here, I don't know where to start.

  • You don't need a GIT administrator to review.
  • Nobody will review 100+ projects per year voluntarily, you realize people have jobs, right?
  • 2000+ projects are not reviewed per year. It doesn't matter if your numbers look good when they don't reflect reality. The reality is that a lot of projects still wait over a year to be accepted.

I mean that there are too

g089h515r806's picture

I mean that there are too many Git managers, but most of them do not do review works.
Even Git managers do not do review work, other people also do not do it.

我的drupal博客Think in Drupal

Can I help?

mherchel's picture

Can I help out with this? What do I need to do that will actually speed things up? I can probably spare about an hour or two a week to review some projects.

Let me know

Hey Mike, Good to see you.

bhosmer's picture

Hey Mike,

Good to see you. Anyone can review projects by going to the project application que and helping out. After you review it, mark it as RTBC and a git admin will eventually come along and either approve, or add suggestions to improve it.

Firefoxes approach does not

bfr's picture

Firefoxes approach does not work here because the purpose is different - while the process is named "project application review", it is actually a developer review. The catch is to make sure the developer is par with drupals coding and security standards. That's why you are only required to go trough the process once. Firefox extensions need to get approved every single time. Drupal.org has 20.000 projects. Imagine all those in the issue queue.

Set some limits

fizk's picture

If too many reviews (or reviewers) per application is becoming a serious problem, we could set a limit on either:

  1. The number of reviewers per application
  2. The number of reviews per application

1 could help spread out the reviewers across more applications. It would likely also decrease the amount of reviews per application since there would be fewer people submitting reviews per application.

I don't like #2 because an application should have an infinite amount of reviews until its ready.

There is no such thing as

klausi's picture

There is no such thing as "too many reviews". Getting a review is hard and if you get many of them be proud, that means your project is interesting. If a review is short or of low quality it is also easy for you to handle: not many things will need to be fixed, and you can set your application back to "needs review" instantly.

So... If every time you fix

fuzzy76's picture

So... If every time you fix all issues a reviewer has, a new reviewer pops up with a different list of issues, and this makes your chase an ever-moving target of "good enough" you'll never reach, this is a good thing? My example above shows an applicant that has satisfied EIGHT reviewers, used 17 MONTHS and still isn't able to get in. While in the same period, a lot of projects has been let through with only one review.

How is that fair?

Removing review bonus tags

Frank Ralf's picture

after the applicant already did two rounds of reviews with seven reviews!

Looks RTBC to me now! Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

http://drupal.org/node/1812542

So there not only seems to be no such thing as "too many reviews" but also nothing like "enough reviews" ... I won't comment on this any further.

I wasn't talking about review

fuzzy76's picture

I wasn't talking about review bonus at all in this context. The applicant has not done any reviews of his own. He received 8 reviews on his own module, each one just when he satisfied the issues in the previous review.

Yes, but

Frank Ralf's picture

you see the applicant in my post did satisfy all issues regarding his own module (as it is RTBC) but even so has to do additional three reviews to finally get his code approved.

If that is how review bonus

fuzzy76's picture

If that is how review bonus is supposed to work, I'm nothing but shocked. Approved code can be stuck for months even after it has been reviewed and all issues resolved? That's blackmail, not bonus.

Not required

sreynen's picture

I think it's important to clarify that the review bonus system is entirely optional, not required. It's not the case that anyone "has to" do additional reviews. That's simply an option for getting some people's (e.g. klausi's) attention faster. Other people (e.g. myself) give attention to applications for completely different reasons. klausi's attention happens to be very valuable because he's incredibly active in reviews, but that can easily change.

If you don't like the review bonus system, there's an easy way to make it less relevant: do reviews yourself without looking at bonuses. If you think people who have been waiting longer or people who have had more reviews deserve more attention, you don't have to convince anyone else you're right. You can simply give them your own attention.

Inciting nitpicking

Frank Ralf's picture

I agree. The current process might increase the quantity of reviews but not their quality.

And IMHO a page like "Increasing efficiency in manual code reviews" actually makes matters worse as it makes it easy to create an important looking review by just copying & pasting without actually testing the code.

See also "Ventral even more nit-picky than Coder's strictest settings" for some more thoughts on this subject.

It comes down to you

fr3shw3b's picture

Where this is such a concern I think it comes down to each individual reviewing and their motives. Where people are going through the Project Application Review process it is because they take things seriously and so do the reviewers. To deal with the supposed problem of poor reviews from those seeking to get a quick review bonus, it's not too much of a problem. In the process for me I had one or two of these and in the long term it helped as I researched into the a particular vague review I had with no real direction of what to do. The amount of time it takes you would assume usually depends on the dedication of the candidate and the reviewers and their research beforehand into creating and reviewing quality projects. Do more "quality reviews" if it's such an issue. Increasing quality in reviews would be through reviewers willing to learn more by learning from reviews by Code Review Admins especially and less people discussing the current problems and just getting involved. It's best to just be the person willing to help that person who still hasn't been approved after a long period of time. Review Bonus is excellent because it's caused me to want to do reviews and learn a whole lot more and want to get to a point of the highest quality I can possibly deliver.

That's my say.

I just completed the process - a few remarks

sashainparis's picture

Hi all,

As I just completed the process, I have a few comments to make:

1) I have a pretty complicated module project: I decided to wait for more maturity before showing code for 2 reasons. Reputation is the first ;-) The second one is I had the feeling that submitting a simple module would be a good idea to have commit permission quicker. Obviously, it was founded.
I believe that the worst story about this page concern D.O. member who submit too big modules to application process - so I would advice them to withdraw this 1800 lines application and write a 300 lines modules with 2 files, 8 functions and 3 hooks.

2) Concerning the Review Bonus, I really feel that you can hardly go without it but: don't burn it for nothing as it is pretty expensive if you try to make 3 value added reviews for other members!
But I would advice not to use it before you feel your code is ready to get the RTBC status... or that you already have it.
I saw several application issues where the member made this 3 times. Whooouh. OK if you are a student and you have time. If you don't have time to make 3 reviews again, your application might stay here for weeks :-(

3) Let's be honest, to pick a module to review I had 3 criteria: Is it simple or pretty complicated? Am I the right person to bring value added and advices to the developer of this module? Does this modules interest me? Everyone prefers to spend time on something he is interesting in, right!?
As a consequence, if your module is to complicated, innovating, dedicated to a specific library... You will have less person to review it. Sad but true :-(
On my own module, once I had 8 comments and a PAReview Security tag, the game was over. No one stepped by until I published my PAReview Bonus.

4) So did I learned something: I do Drupal for a few years, now and yes I did learn a few pretty value added details through this process. As a consequence, I like it. But 4 weeks to go through the process (OK: including Xmas and new year holidays...) for that kind of module was not so short.

To react to the last comments: I feel switching from RTBC to Fixed is not that long. So I understand Commit Admins answers. From their part, it seems to be OK.
I have the feeling that most of people who make the reviews are newcomers that are applicants - so they might not be very comfortable with switching status to RTBC. The other reason is that, well, they are hunting modules to review for obtaining the PAReview Bonus, man! A manual review that has nothing more to say that: 'This is OK for me, nothing to add, here's your RTBC" might not be taken into account for the Bonus list. This is pretty perverse, and it doesn't really shorten the review validation process :-(

Maybe what we need is experienced reviewers between newcomers and admins that would more easily take the responsibility to say: "OK ! There is this detail that would be nice to be changed but you did prove you understand and apply Community standards." And switch status to RTBC. Why would they do it more easily? Because they are not hunting the Bonus anymore :-D
Let's call them "Application Angels" - it sounds cool to me.

If you are interested by this approach, have Commit permission and want to be identified as an Application Angel : please, answer to THIS comment. It would be nice to have a page to list them too.

Alexandre

Thanks for this write-up

Frank Ralf's picture

That pretty much summes it up.

IMHO the crucial part is getting those people to review a project which are genuinely interested in the module's features, not doing it merely for getting their bonus points.

So to repeat my suggestion:

Make sandbox modules easier to find

IMO this should involve changing most of the current features of sandbox projects:

  1. Don't hide them in the attic but make them more prominent (see my suggestions above).
  2. Mark them appropriately but don't give them a warning as if the code were dangerous. If for example the Media module can get away with an 7.x-2.0-unstable7 release I see no reason why the community must be protected from - mostly harmless - sandbox project. Security issues should be one of the few main concerns in a formal review process. Everything else can very well be handled by the community.
  3. Give them a human readable short name so they are more easily findable.
  4. Create releases for download so also people without any Git knowledge can test them.
  5. Show them in the Main project issue drop-down.

Sandbox vs Project?

sashainparis's picture

Frank,

What would be concretely the difference between this new Sandbox and a Community project?
I believe the present difference makes it clear drupal.org is not github-like. It doesn't encourage to fork and adapt easily, etc.

Are you wishing some more github-like platform approach?
Why not - but I believe this change strongly the Drupal Community governance...

Alexandre

It shouldn't have to be 100% perfect

rooby's picture

I think one problem is the need for perfection.

Something that seems to be a problem is that sometime people seem to want to make issues out of nothing so it seems like they have done a good job with the review bonus (or something).
I have even seen reviews saying that good code should be changed to bad code because the reviewer didn't actually know what they were talking about.
Also, some of these issues turn into an issue queue where people are suggesting new features and unrelated architectural changes etc., which is not the point of it at all, that part can come later when the project has been made (or in the sandbox issue queue).

I can look through someones module and it might be perfect except for two whitespace (< look a white space error :)) errors and a mistake in a comment out of 5000 lines of code.
Now I know you could argue that it is easy for them to fix those problems, but come on. It means back and forth, and it means reviewer time has to be spent coming back and re-reviewing when it is pretty obvious the coder knows their stuff.

Once it is obvious that the coder knows what they are doing with their coding and the drupal API and can adhere to the standards that should be sufficient. Why go back and forth with tiny little things, which would be better fixed in the issue queue after the project is created, instead of holding up the reviewing resources.

I was under the impression that Drupal was of the motto that it is better to get your code out now and then work on perfecting it as opposed to waste time trying to get it perfect from the beginning, all the while (the majority of) people can't use your code. (see http://webchick.net/embrace-the-chaos - and many other pillars of the drupal community have said similar things across the web as some point.)

Again you could argue that the sandbox is getting the code out now and that the review process is the work on perfecting it later, but the reality is that mainstream/less technical users aren't going to use sandboxes, especially if you need to use git to get them, and look at how many not-perfect full projects that are out there that are fantastic and used by many thousands.
In my opinion people would be more likely to use a full project with a dev release and no stable or recommended releases over a sandbox.

The review process seems to want perfection before the module can become a full project, and that seems like it's wasting a lot of peoples time.

Also, a side note on:
"(NB: No sandbox module is listed on http://drupalmodules.com which IMHO is still far better for finding modules than drupal.org.)"

I don't know why you aren't using google to search for almost anything on the web, including drupal modules. Google has far better search and searches sandboxes & full projects.

However once I find a sandbox I am reluctant to use it unless I really have to, because they are often unsupported and have no security releases etc. (I know, lots of full projects have the same issue, but it seems safer to me, and I'm sure it seems a lot safer to non-developer types).

Sorry for somewhat repeating myself, it's late :)

Thanks for your thorough reply

Frank Ralf's picture

and for the pointer to webchick's article!

Very True

mishradileep's picture

Very True Rooby,

You raise all the points in very decent manner.
I agree with your point that community should be for push you and others towards perfect.
What I can do right now is just request to all my community friends to think on purpose of review and try to achieve it, instead of sticking on the rules. Rules are made for manage the process and if rules are disturbing our process it should be modified accordingly.

I'd like to add my two cents

jnicola's picture

I'd like to add my two cents since I just went through the process.

There were MANY times where it frustrated the living piss out of me. I've been working in Drupal since 4.7, and am just now contributing a module. Drupal has come a long way, but sometimes I feel the community is a bit high on itself, and a lot of these processes feel like an ego circle jerk to me.

However, for all of my frustration, I got a lot of distinct good out of this process. Overall, I feel like my coding standards are now higher, my knowledge increased, and overall I think it's been a beneficial process... even if it took a month and a half to approve my silly simple module.

Still though, I dig the process. I got to see what others were doing, and review their code. It forced me to think more about how Drupal works in reviewing it. I also got a good amount of good feedback which I learned from. It also forced me to think out solutions to things that weren't really major problems, but needed to be resolved (unix EOL converson in notepad++, etc etc).

Some suggestions I do have:

  • It's hard as a new module contributor to review others modules and feel confident marking it "needs review" or accepted it as tested... because you just don't know a lot yet. My early bonus reviews I didn't mark the module I reviewed due to distinctly not having enough knowledge. It would be nice to encourage this practice.
  • A -LOT- of modules I tested were getting glowing code reviews, but when you enabled the thing in Drupal it didn't actually work... I know this is about code review, but still... the thing should at least WORK! Even more frustrating, i don't think anyone installed my module to test it.
  • I saw a lot of "features requests" in others reviews. The issue pointed out didn't cause the module to not function (I know, I actually tested all modules on a D7 instance) and it didn't have anything to do with security... these I feel should be posted in the review, but not result in "needs work".
  • I agree with the point about people giving incorrect advice! I was told I had to do all this stuff to sanitize input and output... and then Klausi came through and pointed out how redundant it was! Grantd, I learned a lot from the mistake and improving from it, but still...
  • The automatic code review was somehow a surprise to me. Overall, the whole process is a bit daunting and I'd like to see just one single document covering the important basics everyone needs to know. I suppose I could just go update the documentation though!

Jesse Nicola -- Shredical six different ways to Sunday! -- My Portfolio

Does a paid review bounty

jrabeemer's picture

Does a paid review bounty seem like a good idea? We're all busy people. If someone say, offered $20-30 for 30 mins of sandbox review time and gave a good thorough review, I'd have no problem with that.

I don't think that will

fizk's picture

I don't think that will happen on Drupal.org, but there's nothing stopping people from making these kinds of arrangements in private.

This to me is a bad idea to

jnicola's picture

This to me is a bad idea to encourage at all in my opinion. It's already almost standard to get your module moving in even a semi timely fashion to do the bonus review, I'd hate to have to make it come down to money!

Jesse Nicola -- Shredical six different ways to Sunday! -- My Portfolio

I'd just say "give me the

fuzzy76's picture

I'd just say "give me the money, and I'll make you comaintainer on one of my modules". That way, he gets his git access and I don't have to look at his code. Problem solved. :)

Very frustrating review process.

thomas_rendleman's picture

I have spent over 100 hours to do the graphics for a module. It has every flag for every country and the U.S. has a flag for each state as well. I didn't need to make 246 extra flags X 2 backgrounds, animated. It took over 100 hours just for that alone. I can't seem to get anyone to review it with any detail and mark approve it to be a full project.

I think that such a delay in the process of approving or reviewing takes the joy out of writing a module. Why did I spend all this time to be ignored?

Some context

sreynen's picture

I don't think it's accurate to say your application has been ignored. Your application has been open for 7 days and it has had 2 reviews. It has most recently been in "needs review" status for 1 day. There's a lot of room for improvement in this process, but I think your application is actually an example of things working about as well as they can. I'm sorry you're frustrated, but I hope you can appreciate that it's also frustrating for volunteer reviewers like myself that even a 30 minute turnaround from "needs review" to a review (which happened on your application) is not fast enough.

For context, there's currently an application that has been awaiting review for over 5 weeks. Maybe you could review that to help speed up the process?

Time to revisit and BUMP.

Diogenes's picture

I wanted to change this wiki page back to it's original title "Module Approval Process will KILL Drupal" but I can't seem to do that.

I think it would be better if the title was "The Module Approval Process is KILLING Drupal".

There have been many ideas posted here; good and bad. Not much progress or change is evident since the first post almost two years ago.

Maybe this is not the best way to do things. But neither is creating another discussion that will be forgotten if it is not ignored.

Four simple suggestions...

Diogenes's picture

1) Create a simple code review system that provides a simple compressed (non duplicate) warnings list with simple metric and graphical indicators to indicate the level of risk. Run this on every project without exception. Different warning levels would be good.

2) Provide a means for users to review modules and allow others users to rate the and comment on those reviews. Amazon style "helpful" or "unhelpful" aggregate ratings will be fine.

3) Give every existing contributed module that needs cleanup 6 months to a year to do so. Automatic promotion or delisting after expiry.

4) A serious discussion on the difference between "coding standards" and "best practises". There are differences.

There is another thread on

Perignon's picture

There is another thread on this. I don't have it handy as I am replying from my phone.

The Module Approval Process is KILLING Drupal

ngocketit's picture

I totally agree about this. I made a simple module that I think maybe useful to other people as well but the application process has been taking half a year and my project is still a sandbox, which discourages me from sharing my further modules.

A few people have stepped up

mpdonadio's picture

A few people have stepped up to help out with the process, but it will help you (and others out) if you can participate in the Review Bonus program.

Pravin Ajaaz's picture

Once we reach RTBC and we got review bonus in our application there is no way we can proceed. The application still resides in the queue for a week. There might another 3 review bonus to improve the situation since it's already RTBC (after a long wait) people might find it frustrating. Should add a special bonus for RTBC to Fixed. something like for finding a security issue or such

Peer Review

Group organizers

Group categories

Project Type

Group notifications

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