How much code do we need to approve a user?

We encourage users to post events happening in the community to the community events group on https://www.drupal.org.
klausi's picture

The project application review process is done only once per applicant, so we need to make sure that they provide us with enough self-written code that we can review. Otherwise we cannot really judge their skills and understanding of Drupal.

This does not mean that short projects will never get approved, a git admin can promote a project on behalf of the user without giving the git vetted user role to them. We decided to do so also for simple Features only projects, so we could also do it in these cases. Applicants can come back to the review queue whenever they have something new they want to promote.

I propose two simple rules that can be used to deny the "create full projects" permission:
* The project has less than five function or class method definitions
* The PHP/Javscript source code is less than 120 lines long (including comments and whitespace).

This does not mean that projects that satisfy these rules will automatically result in the "create full projects" permission. We can always discuss if there are other reasons why a project in not enough for the "create full projects" permission.

Thoughts?

Update 2011-12-24: Added Javascript source code. Tried to clarify the rules a bit.

Comments

I think it's largely a

jthorson's picture

I think it's largely a judgment call on behalf of the reviewer and/or git admins. A project might have 120 lines of code with half of that being a views import ... or have five hook functions (hook_install, hook_schema, hook_help, hook_menu, and hook_myonefunction) ... and in both cases, still not provide enough information to truly review the code.

What I tend to do for shorter projects is take a look at the contributor's d.o profile ... if they've been a member for a few years, and have contributed contrib patches and/or helped out others in the forums, then that extra information is usually enough that I'd recommend a full approval. On the other hand, if their account is only weeks/months old, and the only posts in their profile are related to the project being reviewed, then I would tend to apply a much stricter standard.

Yes, but 120 lines and 5

klausi's picture

Yes, but 120 lines and 5 functions is a minimum that we need and we can define this as a general guideline. If it is more than that but still largely copied code that does not indicate the applicant's skills, we can still discuss whether it is acceptable or not.

I'm okay with the values as a

jthorson's picture

I'm okay with the values as a guideline ... I'm just skeptical that applicants tend to interpret 'guidelines' as 'hard and fast rules'. :)

Oh well..

patrickd's picture

Perfect example: http://drupal.org/node/1365872
Isn't putting a module with 8 lines of code not.. a kind of 'brazen'?
Nothing personal, but this really annoys me..

back to topic: I'm absolutely agreeing!

IMHO I also would not do any exceptions for long term members, I think there is still a great difference between contributing patches or modules..

At this point, I disagree.

doitDave's picture

At this point, I disagree. Providing patches is at least as important for the community as new modules are. If not even more important, just think of all that duplication mess. I would even go so far to say that someone who - in whatever way - continuously provides patches of a certain quality may easily prove even more adherence to very basic Drupal ideas (and, of course, collaborative coding in general!) than not just one of the application examples I have seen within few months recently. This is, however, no rule of thumb anyhow, but reading into an existing project, debugging AND patching it with standards compliant code is a task at least as sophisticated as creating something from scratch.

Just my 2 Pfennigs.

(Edit: What I do NOT disagree with is that the example you gave is a good one for a - I will really not excuse for that wording! - lousy application and, additionally, also perfect do demonstrate an attitude I personally would really not like to meet too often in this community: "Pah, if you are unable to recognize my ingeniousness, I am at the wrong place!1!". That actually makes me angry.)

We should have some

frob's picture

We should have some clarification on the guild lines.

Why only PHP a good deal of functionality can be delivered with javascript, and if the javascript is written to the drupal standard should that also be taken into account?

I ask this just as much to document the reason as to find the answer.

The motivation is to help

jthorson's picture

The motivation is to help address one of the goals of this application process ... to ensure that the applicant has demonstrated at least a basic understanding of the Drupal APIs, and in some cases, the 'Drupal Way' of doing things. This would, of course, include the Drupal Javascript API.

In my opinion, the 5 function limit should be trivial even for a javascript module, as even the most basic (but well-written) javascript module will probably end up with hook_help, hook_menu, an access callback, a main PHP function, and the javascript file.

The motivation behind this discussion is that we've had project applications with as few as 8 lines of code ... and it become hard to determine if someone is sharing a module, or simply throwing together a quick hook_alter() function just to get full project rights. (Note: I'm not suggesting this was the motivation behind any recent 'short' applications; only that it could potentially be perceived as such by a reviewer.)

Yes, this totally makes sense

frob's picture

Yes, this totally makes sense and I completely agree. I do think that the bare minimum should be a hard-and-fast rule and not just a guide line. 120 lines of functional code including comments and whitespace with a minimum of 5 functions/methods.

Any programmer should be able to stretch the simplest module out this; or at the very least add additional functionality to a module to make it a bit more useful.

I can't get behind a 'hard

jthorson's picture

I can't get behind a 'hard and fast rule' ... I would hate to have applicants starting to add 'bloat' to their modules, just to meet the 'minimum lines of code' requirement. Ideally, we want to keep this process centered on the 'applicant', not the 'code'.

A better way

v-a-1's picture

It occurs to me that:
1: Existing contrib modules do not always follow coding standards. Just running the coder module on some popular modules on my fresh d6 installation popped up around 200 coding standard issues.
2: Existing contrib modules get abandoned on a regular basis.
3: Modules that are superseded in functionality by newer modules continue to reside on DO, and the only indication of this is if the developer of the original module indicates this by updating the description of the module.
4: The review process takes a long time per module for a variety of reasons (time available to reviewer, reviewers understanding of the module being offered etc.)
5: Limiting git access would simply exacerbate that which obviously is already a challenging process.

Considering all of the above, I propose:
1: Git access should be granted to newer developers irrespective of size and scope of module provided, thus encouraging contributions and reducing per module review load.
2: Git access should be conditional. That is, if a project gets abandoned simply due to a lackadaisical attitude on the developers part, git access for new projects should then be revoked/suspended, until existing projects are fixed.
3: Git access for new projects should be suspended until at least coding issues in existing projects are fixed.
4: Another solution would be to reinstate access, or even grant access if the user participates in say 10 review processes for new modules (as a penalty for malingering).

All of the above would have the following effect:
1: Review load would reduce or at least not increase.
2: Quality of existing modules would continue to remain top notch.
3: We would end up incentivizing participation in the review process.
4: Will keep a fresh breed of developers encouraged enough, which would keep the vitality of DO alive.
5: And most importantly, the people who depend on contrib modules for setting up their websites will continue to find relevant modules for their specific needs when they need them.

The suggestion put forth in the original post is, imho not at all a solution; but rather something that will erode away all that is good about drupal in no time.

@jthorson : I agree that we

frob's picture

@jthorson : I agree that we should encourage developer to not just add bloat to the module to get it in the 5 functions and 120 lines of code. However, this is a manual review process. That means it is easy enough to look at the code and respond with critical review about bloated code. I was told that my own project was not long enough (even though if one counted the JS then it was). So what I am doing is writing drush integration. This would not have gotten done nearly so quickly had I not gone through this process and had this requirement.

I think this should be added to the automated code review scripts.

@voodoo : git access is granted freely. All one must do is request it. It is only the ability to turn a sandbox project into a full project that one must go through this review process, and I am all for that. Although, you do bring up some interesting points.

First I agree that some of the most popular modules having coding issues is a problem. This however could be dealt with by having an automated system that checks newly released updates (not the dev branch) and if anything is spotted an issue is posted in the projects issue que. This is, of course, way off the topic of this thread and should be brought up in its own.

Speaking of bloat: Will

doitDave's picture

Speaking of bloat: Will comments take account into the 120 lines? And, if so: Will they be bloat while, normally, some coders regard comments as the holy spirit of coding? And if yes, how will you judge "bloat comments" apart from "very well, even extraordinary documentation"?

Then: How does

<?php
if ($condition_a && $condition_b || ($condition_c && ($condition_d || $condition_e || $condition_f && ($condition_g || $condition_h) || $condition_i)))
{}
?>

count versus

<?php
if (
 
$condition_a
 
&&
 
$condition_b
 
||
  (
   
$condition_c
   
&&
    (
     
$condition_d
     
||
     
$condition_e
     
||
     
$condition_f
     
&&
      (
       
$condition_g
       
||
       
$condition_h
     
)
      ||
     
$condition_i
   
)
  )
) {}
?>

or versus

<?php
$condition_1
= $condition_g || $condition_h;
$condition_2 = $condition_d || $condition_e || $condition_f && $condition_1;
$condition_3 = $condition_1 || $condition_2;
$condition_4 = $condition_a & $condition_b;
$condition_5 = $condition_4 || $condition_1;

if (... and
so on)
?>

You get what I mean? The first one would not be under "bloat" suspicion. The second one would make more lines ("bloat alert!") but is obviously more readable while #3 (of course with more describing var names, this is an example, man!) is closest to the coding standards recommendations ("split long conditions into steps") but also makes far more lines than #1.

TL;DR - what I want to point out is, that I think we should not put too much efforts in judging strict by whatever "laws". While I really sympathize with the very largest part of all acceleration attempts, we should not turn this into the most important principle. There will always be cases urging to judge, well, by case and not by "law". As well as there will always be applicants complaining on either way of "judgement". This is a dynamic and, more important, individual process by itself. (Which is good, by the way.)

So, let's decide for some internal "plumb lines" but leave the last decision to us humans. If there are borderline cases, we will recognize and, for sure, solve them as well ;)

Thankfully we have coding

tim.plunkett's picture

Thankfully we have coding standards, so your point is moot.

As out point out, the coding

frob's picture

As out point out, the coding standards should take care of this one case (and I would expect most others like it). By setting a minimum to be considered one takes away the option of what it was originally recommended for me to do with my project. It was recommended to me that I submit no code --only a kickass idea. The thought was that this way I would not get bogged down in coding standards issues. This was a few years ago, before the git migration; closer to when I started with Drupal.

It is my opinion that 120 lines of documented functional code (no exportables) including javascript is a good baseline. If a user is supmitting a module that has less than that then let them argue the point in the issue que, just as they must argue why their module is not exactly like every other module out there.

The Rules do not imply that

klausi's picture

The Rules do not imply that longer projects are automatically qualified for the "create full projects" permission. They just state that everything below the limits can never be used to grant the "create full projects" permission. So we can make fast decisions on very short projects.

Maybe I should clarify that in the OP.

I don't know if it is just my

frob's picture

I don't know if it is just my own opinion on the subject, but I will through this out there as well.

The 120 lines is such as short amount that it should exclude exportables. Exported content types, views, or features should not be counted among the 120 lines of code. If this is the case then it should be in the proposed new rules.

@voodoo.child: We cannot give

klausi's picture

@voodoo.child: We cannot give the full project permission to everyone, because that would result in name space squatting. Of course you could argue that you can take projects away from spammers, but that would be a nightmare for users: the code in the project would always change and you could not rely on projects/maintainers anymore as we do now.

Additionally, the security team made a pledge that they look out for all releases of contributed projects on drupal.org. We regularly find security vulnerabilities in project applications, so if those projects would go live immediately the security team would get flooded with reports. And Drupal would of course suffer from a damage to its quite secure image right now.

That's why we need to review your work at least once before we trust you. I think you got the wrong impression that the reviews would only be about coding standards -- they are not.

@klausiI think you were

v-a-1's picture

@klausi
I think you were perfectly clear in the OP.
Sorry, but I think I wasn't clear. Wherever I say git access I meant 'full project access'. And I meant that the existing review process continue the way it is.
What I meant by revoking git access, was actually revoking 'full project access'. The developer continues to retain access to projects he/she currently maintains, but cannot create new projects until problems with existing modules by the developer are fixed.
Maybe this isn't the post to discuss this: but my thinking was that instead of thinking of different ways to filter project submissions and offering per project permissions, we should grant full access and the responsibility that comes with it (after the appropriate review).
If the responsibility is shirked, revoke access until maintenance and other duties are fulfilled. I don't know if this kind of permission juggling is possible.
Thanks for the link to the 'security vulnerabilities' post. I had not considered this earlier.

encouraging participation

Barnettech's picture

I actually think there should be a minimum number of functions to prove competency with Drupal, but with encouraging remarks, and fostering further participation.

I agree with voodoo that this group needs to encourage participation, not discourage it, while still enforcing standards and ensuring compentency. There is quite a bit of focus on making sure folks don't get access, while the goal should be to foster the skills for more to participate. I am a new participant in this group. I did not mean to get off on the wrong foot by asking questions about the coding standards, and by pointing out that most modules out there don't fully comply even with the coder module in "normal" mode. I tried to show my enthusiasm to participate that I had come to understand why some of the coding rules exist so sent out some emails using the contact form in Drupal. I don't want to do anything but enthusiastically participate ... I encourage reviewers to patiently teach, and welcome new folks into the community. Remember the highest ideal is to harness incoming enthusiasm, answer questions, and assimilate new talent (and reviewers) into the Drupal community. I do agree with voodoo.child that the object IS NOT to deny access to git and participation. I think modules should be of a minimum length to get awarded git access for first time entrants, but the goal IS to get them into the fold, not the other way around.

I have a team of folks I help lead and since starting my review process (http://drupal.org/node/1328366) have started requiring my team to go over the drupal.org coding standards... there's actually a lot to learn from the process -- best practices, etc. Anyhow, I think having a welcoming attitude and fostering participation is the most important thing.

I liked voodoo.child's comments about encouraging folks and encouraging ongoing maintaining of modules.

We're currently having the problem that the core forum module doesn't comply with standards and most of the output does not go through a theming function or tpl file. We plan on filing patches for this.

We have a team watching these threads and we're looking to participate. Currently my pointing out that most modules don't fully comply with standards, seems to have put my review process on hold -- well some have threatened folks wont want to review it. I am entirely sorry if I offended anyone by challenging their reviews, asking why my spacing for lines was holding up my review process while great modules like Forums, Views, Migrate, and other key modules have far worse infractions. I also followed up with some emails to reviewers (using drupal's contact form) and got flattened for my enthusiasm in showing reviewers I was learning their ways. I do understand that once you get git access you can post projects without review again, I did not know that and now understand why some reviewers are quite rigid.

But now my team which has been reading up on the drupal coding standards, is shy to review my project, a bit afraid of this group. I ask that questions be taken as a learning process, that participation be encouraged, that mistakes be expected along the learning curve.

I have started helping review a few projects myself, and hope to do a good job.

At the Boston Meetup we're exploring ways to help with reviewing core bugs, patches, etc. I've made some suggestions, and hopefully our Boston group can help really make an impact on Drupal's progress.

Anyhow I don't mean to call out the nice folks who did volunteer their time to start reviewing my module -- but I do want folks to remember people learn, ask questions, make mistakes, and iterate along the path to success. I hope you'll welcome me and my team to get git access, and to begin helping with reviews, bug fixing, etc. My biggest suggestion I'm going to make for the Boston Meetup is that there be a mentoring process. There are some pretty heavy hitters at the Boston Meetup and they are overburdened in how much they are needed.

Anyhow I hope you'll review http://drupal.org/node/1328366 and feel free to be in touch if you need my set of eyes to review something for the RTBC status to help move Drupal forward.

Just my 2 cents. Don't hate me. I don't want to be forced into the Wordpress community ;) I much prefer Drupal.

So while showing a minimum number of functions and proficiency might be appropriate, I hope we all remember to encourage participation.

Barnettech

I'm going to add my

bhosmer's picture

I'm going to add my perspective here from the other side as a new project applicant.

I actually went through this process a while ago with a separate module that added a quick way to clear the cache with a button on the admin bar.

My application was denied, because the reviewer said the devel module handled that already. I disagreed with the justification because the devel module was much heavier. Mine was created to do one thing and do it well.

In the end, while I did disagree, I submitted to the authority and didn't push the issue. I moved my project to github instead.

Now recently, after participating in Klausi's PAReview process, I came across another application that did almost the exact same thing. I've joined in in helped the applicant with some bugs and I think he will eventually gain full-project access.

This was the frustrating part. Both my original module and his new one would be useful. I really hope one of them gets promoted at least.

I've been an active member of the community for a while. My goal isn't just to get full-project access. I only ask for it when I think something would be useful to others and helpful and beneficial to drupal as a whole.

Now on to my present application:

The review process has been very educational. I've gained a lot of knowledge from it and have learned a lot about coding standards and the drupal way.

My current module is pretty small, does one thing that is very specific, but there isn't another module that does anything similar this time.

The review process is a good thing and I fully recognize and support the foundation and ideals behind it. I respect the reviewers and welcome their criticisms. I want my module to be secure, useful, and as bug-free as possible.

We should be careful though about being too strict. This will discourage participation and new ideas and new blood.

When I see a module like this: http://drupal.org/project/rickroll "The Rick Roll" module, it definitely makes the full-project clique seem, well, clique-ish.

It left me wondering why my original application was denied, but also questioning the usefulness of the Rick Roll module. (Right now one site reports that it is installed and its usefulness is questionable.) I'm not saying anything bad about the maintainer though. He is definitely a contributing member of the community.

The community is trending toward smaller, more maintainable modules and code. If the process becomes impossible for a new applicant, github might become the new drupal.org/project. This would not be good for the community at all.

Thanks for that post

Diogenes's picture

I had not read this discussion before and I just finished reading most of it. Interesting points of view regarding minimum size.

I just attempted to find a module that did a simple thing -- add Twitter, & Facebook (maybe more) share buttons to a page. Attempting to find something like that on Drupal.org is not easy because:
1) There are so many choices that come up in searches
2) Many modules have very sparse information on what they actually do
3) There are so many rubbish and cruft modules on Drupal.org that it is fast becoming a joke.

I tried ShareThis and then Easy Social. The ShareThis module had no readme file and only minimal information. After installing and enabling it absolutely nothing showed up the display. There was all kinds of HTML markup generated but no character data or images that a real user could actually see, just empty 's

I guess you need some kind of 3rd party stuff here.

Easy Social looked really good. After installing and enabling, I spent the better part of a day trying to get it to work properly and figuring out why it didn't work. This is a textbook case of code bloat. Many of the options don't work yet. It's a work in progress - should be alpha with all kinds of warnings - not a recommended release.

I'm using it right now in it's default config because that's the only one I could get working without a bunch of hacking. I hate it. Tomorrow I plan to write a node.tpl.php file that will do exactly what I want and will take significantly less time than I wasted attempting to get these two modules working.

I'm using a couple of other modules that have have become bloated beyond all reason. They both have their quirks. Try to tweak either of these is a challenge because there is so much code.

Someone said that in every large program is a dozen little programs that want to be set free.

Simplicity is a good thing. The Drupal mantra of "collaboration, not duplication", as it is currently applied, effectively defeats this principle. Drupal is going to lose more valuable members than it gains if there are no changes.

co.no.du.

carwin's picture

@Diogenes: This is sort of in line with what you're saying, but I wanted to add that one of the problems I see occurring with the "collaboration, not duplication" concept is that at some point there will be a legacy module, let's say "Project A" which was created for Drupal 5 and has slowly just been upgraded up to, say, Drupal 7.

[theoretical stuff]
So let's say that module is just utter garbage. The implementation of the concept is horrible, the code is sub-par and the thing just doesn't work.

Project B comes along in the queue and does the same thing, but way better. Should we force Project B to "collaborate" with Project A's crummy work simply because it's older? What if Project A's maintainer hasn't been seen in two years or just got his access grandfathered in and never went through the current review process?

In the first scenario he could apply for maintainer access for Project A and after some time report the module as dead and be given ownership without the approval of the missing maintainer. After that he's now got the entire history of garbage code to work with - which he could just re-write and release as a new version. That whole scenario could take a month, maybe more.

In the second scenario, you now have two maintainers of a project, one of which has had his project for years and will probably be a little resistant to the changes the maintainer for Project B proposes. This case may or may not really arise because every person is different - but the possibility is there. It's unfortunate that Project B's maintainer would just be fighting to get something working in a collaborative way that already works and exists in his own project.


I couldn't agree more about the fact that there is a ton of cruft that needs to be cleared out from the d.o project repositories, and luckily I think there's a team for that.

For my part, regarding the social network links, I think those modules should be kicked off d.o entirely because their entire functionality can basically be covered by adding a block and copying the ready-made embed code from the provider into it.

In fact, here you go I didn't even need to create an account for ShareThis:

<!-- put in template -->
<script type="text/javascript" src="http://w.sharethis.com/button/buttons.js"></script>
<script type="text/javascript">stLight.options({publisher: "ur-4d79f1ab-c5a3-659e-73d6-894dd9e1f19"});</script>
<!-- put in the block -->
<span class='st_fblike_hcount'></span>
<span class='st_twitter_hcount'></span>
<span class='st_plusone_hcount'></span>
<span class='st_email_hcount'></span>
<span class='st_sharethis_hcount'></span>

What I'm getting at is that those modules provide a "turn it on" solution, but what they do is so easy to accomplish outside of them that I don't see a point.

A good module is a 5 minute solution

Diogenes's picture

I'm a bit embarassed about how much time I wasted on something so simple - The Easy Social module is a nice concept because FB, Twitter and Google have all kinds of sizes and styles to choose from. But many Easy Social options don't work yet and they got carried away with stuff.

It was so close yet so far away.

I couldn't agree more about the fact that there is a ton of cruft that needs to be cleared out from the d.o project repositories, and luckily I think there's a team for that.

There is a team for that? That is a huge job.

I think some kind of voting system is needed, something like the Stackoverflow model.

some kind of voting system is

beanluc's picture

some kind of voting system is needed

Indeed, it's coming soon. Not just votes/ratings but user-reviews too:
The plan for Project reviews/ratings etc. is now formulated here: http://drupal.org/node/1580230 with active issues #1637534: Implement project reviews and #50605: Add user ratings for projects.

It won't be like Stackoverflow but it will be suitable.

Hi,I've been trying to get

bvanmeurs's picture

Hi,

The reviewing process is over-regulated.

I've been trying to get full project permissions for over a year now. My code quality is good and I've been working with Drupal 7 for 32 hours a week since it came out. I've noticed and fixed core bugs and have an higher education in computer science.

Yet, despite several tries, it has been IMPOSSIBLE for me to get full project permissions. A little summary of my attempts:
XSSecurity (http://drupal.org/node/1512124)
The module was thoroughly reviewed but when I found a design flaw in the module I had to abandon it. As the code quality was reviewed already, I thought I could get the permission anyway but it wasn't granted.
Block Position (http://drupal.org/node/1702578)
A very handy module but I stopped trying to get it to a full project when someone asked me to study another (unrelated) project and explain the differences.
Show Inaccessible Menu Link (http://drupal.org/node/1842466)
Because both projects above were quite big, when I experienced a lacking functionality in the menu system I decided to have a go at releasing it as a small sub module. After making several asked modifications I was suddenly pointed at additional requirements unknown and unmentioned before.

I'm about to give up guys. In practice I don't have the time to spend my boss his time on strange requirements. I guess this is the same for a lot of other good developers trying to do something for the community. I understand that you want to have some reviewing, but you are scaring new developers off big time.

I've seen full projects on drupal.org which code quality is very poor. They obviously didn't get the thorough reviews I'm experiencing. Also, when you finally have the permission you can release any crap you like.

I was discussing the issue of bad projects with my colleagues yesterday. For instance the redirect module, broadly used, contains a CRITICAL flaw which will break website with a very simple action (renaming a node to a new title, and then renaming it back to the original title). Still the usage figures (usually a good indicator) indicates the module works fine.

I think it would be better to give everyone the permission to create full projects, and then have a better live per-project reviewing system. Up- or downvoting comments like on the modern forums will ensure that the most relevant usage review information will be shown first to anyone interested in the module.

Sorry to hear your

klausi's picture

Sorry to hear your frustration. Just mention your contributions in the issue summary of your application and I'm sure we can grant you the git vetted user role although the project on its own might be a bit short. Don't let that hint discourage you!

We are all busy people and I have to select from the many applications that I want to review. Follow the review bonus and I will be at your service. After all we are a community and we help each other.

I think the review process has improved newly created full projects a lot in the last year(s), so we slowly kill the crap on drupal.org. Of course there will always be bugs, but bugs can be fixed. No software is perfect, but fortunately we have free software here, so we can simply fix and tweak it to our needs.

Don't give up, I think you are almost there.

Thanks for the support

bvanmeurs's picture

Thanks for the support Klausi!

I'm expecting to have some more time for Drupal development in a couple of weeks time, and I will certainly go and do some reviews!

Thank you for the honest

jthorson's picture

Thank you for the honest feedback ... just as a note, this is likely to be one of the top priorities for a new technical policy working group that is being established over the next few months. Feedback like this will be invaluable to helping them understand the frustration behind some of the hurdles we impose on new contributors.

I'm also sad to hear your

greggles's picture

I'm also sad to hear your frustration.

One of the problems with the process is that it's hard to find people who are able and knowledgable enough to review the more complex applications (and who have time and motivation). The current crew of reviewers is doing great work, but there's just so many applications that it's a tough challenge. The XSSecurity module is an example of a module that is complex enough to be hard to review. I think the application also shows the value of peer feedback - you tried out an interesting solution to the problem but as you say the design was flawed. The review process found that flaw, but would the design flaw be found without the review process?

Baffeled by The Application Process

9ee1's picture

I have been using Drupal for several years now and have developed quite a few modules for my projects. I am a new contributor though and I am hoping to contribute a lot to with time.

One thing that has baffeled me with the application process though is, if the purpose behind it is to assess people's skill like stated, how is that the same people that are being assessed are being asked to assess other people's code to gain priority for their own project's review?

That makes absolutely no sense to me. So if someone is a poor developer, the process is essentially saying "review other people's code using your poor skills, so that when we review your code, we will deny it because of the poor condition it is in.".

Can someone please help explain this to me? And I mean no offense to all the great people spending their time contributing. I have great respect for each of you because it is because of you Drupal exists.

There are really two aspects

jthorson's picture

There are really two aspects which can help answer your question.

The first part is that peer code review is an excellent learning opportunity for developers. I can honestly say that I have learned more Drupal while doing code reviews than I have in any of my own projects. Given the variety of projects which come through the project application process, long-term reviewers end up exposed to a the full array of Drupal APIs and use cases, and often discover new-and-better methods of solving problems that they themselves have encountered over time. So I would rephrase your quote ... "Review other's peoples code, learn from it, and use those learnings to improve your own projects."

The second part of the answer is related to a little bit of the history of the "projectapplication" queue. While the situation today does seem dragged out and onerous (and I would tend to agree with you there), it has taken a few steps forward from when I came through the queue about 18 months ago. Back then, there was only a small handful of fully qualified reviewers looking at the queue, much like today. That said, these were the ONLY people looking at the queue, which was also twice as large as today's queue. As a result, it was quite common that applicants would wait upwards of 4 weeks or more in between reviews. The reviewers would open the project, find that i) they cannot read the code due to custom indentation or code style, ii) the code is full of typos and obviously hasn't been tested, iii) total lack of docblocks and/or in-code documentation, iv) obvious lack of understanding or misuse of the Drupal APIs, and v) numerous security vulnerabilities in the first few functions. Any one of these makes it extremely hard to give a project an impartial review, so the reviewers (who were volunteering up to an hour of their time per project review) would ask the applicant to fix their coding style, mark the application 'needs work', and move on to find another project that was more worth their volunteer time. Meanwhile, the applicant's project would go back to the end of the queue, and they would wait another four weeks or more before it was looked at again.

This process was frustrating to both parties ... reviewers would spend more time looking for a 'finished' project to review than actually reviewing, and applicants would wait long periods of time for nothing more than a cursory glance before having to start the wait all over again.

What initiatives like the 'review bonus' program have done is bring a large number of other reviewers into the queue. Granted, these reviewers may not have the background knowledge needed to provide a fully qualified review to a project ... but they are catching the 'obvious' items and allowing the qualified reviewers (which there is still a shortage of) to focus their time on more 'polished' projects. As a result, I'd speculate that the average time to approval has been cut in half compared to what it was 18 months ago, and half again for those participating in the review bonus program (which, incidently, is not mandatory; but is used by one of our most proliferent reviewers to prioritize where he spends his volunteer time).

Is it perfect? Definately not ... and we still have a long way to go. But the principles being used in the projectapplication queue do mirror many of those used in the greater Drupal community:

  1. Peer Review: no patches get into Drupal core before passing through an extensive and highly critical peer review process (with some larger patches often exceeding 300 comments before finally being marked 'RTBC'),
  2. do-ocracy: Those who 'do' are recognized, appreciated, and rewarded for their actions, and
  3. Collaboration: Everything we do, we do to help each other. No matter how trivial the comment or criticism, or the tone of the statement, step back and ask if there is anything to be learned from it ... and even if not, appreciate the fact that the poster set aside a minute of their own time to try and help your project forward.

Thank You For The Detailed Answer

9ee1's picture

Thank you for taking the time to provide a detailed answer.

I see where you're coming from. Since I am a new contributor, I can't debate whether the new system is better or worse than before.

But quite honestly, with the current system, there is a lack of confidence in taking feedback from a reviewer who themselves are not experienced. Where is the guarantee, or confidence level, that the feedback you're given provides a better approach then the one you've taken, or even worse, if the feedback is even correct?

That confidence can only be gained if the feedback is given from an experienced developer. So now the question becomes, If I choose to ignore the feedback from an unexperienced developer, because I do not totally agree with it, will more experienced reviews take that into consideration when reviewing my application? Or will it result in further delay?

If you can explain yourself

jthorson's picture

If you can explain yourself in the issue thread, it will show that you've considered the advice, and chosen a different approach ... which should be enough for most reviewers.

However, I know that my first step is to go through the issue and make sure the applicant has addressed the most recent comments ... so I wouldn't advise simply ignoring the review without at least providing a comment as to why the change isn't being made.

Clear Now

9ee1's picture

Thanks for clarifying everything. It is very much appreciated.

Unapproved developers doing reviews

beanluc's picture

Encouraging applicants to do reviews does at least two things for them, and for the community.

For applicants:
* Provides motivation and opportunity, beyond their own application, to be exposed to "the Drupal ways", which yields a more rounded Drupal developer.
* Provides a means of expediting their own application.

For the community:
* The same two as above, maybe in reverse. Gains additional application reviewers, in a crowded queue.
* Gains new approved developers who are credible contributors, demonstrably versed in Drupal technology and conventions.

There's no harm in inviting a review from someone who's not necessarily already an approved developer. The risk that they would submit a terrible review is both low likelihood (they're motivated to be as thoughtful as they can) and low impact (so what if the review is poor? It's just another teachable moment). The review process would catch a poor review as effectively as catching a poor application.

To Raise an issue

Perignon's picture

I want to raise an issue. This is my first stab at doing so and I hinged on this post as it is linked from many posts in the project application queue. So...

My issue is the proverbial "kettle calling the pot black". I personally am trying to contribute something that I finally feel worthy of the community after two years of neck breaking development with Drupal to produce a tremendously huge eCommerce site departing from another CMS that was horrible to say the least. I digress... In this process. I recently added some code to be more "helpful" with my module - literally I added a hook_help! In doing so I opened a highly, highly, highly used module on Drupal (LoginToboggan) as it was the first thing that showed up in my search of hook_help in my project. I thought, "cool good module to use as a template". Committed the code two nights ago and instantly had PAReview.sh errors I never had prior to implementing that code.

So... "Kettle call the pot black". I am all for rules but let us be fair. If you are going to impose them on new contributors you should very well impose them on Everyone! Make PAReview scan all established repo's for projects on D.O. They don't follow regulations then the project gets unpublished or put back into a sandbox.

I work by day in a highly secure environment and it doesn't matter if something has been used for 2 decades (hell 30 years!), if it falls below current regulations and code, it is taken offline. Lets do the same here. New developers to Drupal are going to look at well established modules to develop their code.

Case in point. Please look at this!
http://pareview.sh/pareview/httpgitdrupalorgprojectlogintoboggangit

Would this module ever pass trials to be a project? Over 130 errors in the .module alone!

I will be fixing my code. My only issue here is I copied a very high profile module to develop my code. The current code policy flagged my module. No problem at all, but lets flag all the other bad code out there not the new code. As it is blatantly apparent that the well established hard-core modules that people rely on for Drupal installations fall far below the current coding standards.

Allowing this type of behavior to exist does NOT help Drupal community. It hurts it as a whole. All I am asking is fairness. It's time to be a bully and enforce coding standards on everything.

Off my soap-box and onto correcting my mistakes. Thank you for your consideration!!!

Hi Perignon, I totally

bvanmeurs's picture

Hi Perignon,

I totally understand your frustration, but send me a link to your soapbox and I'll review.

That is one hell of a funny

jnicola's picture

That is one hell of a funny typo :D

Sandbox!

Jesse Nicola -- Shredical six different ways to Sunday! -- My Portfolio

HAHAHAHAH It is! I really

Perignon's picture

HAHAHAHAH It is!

I really don't mean to be an a$$hole.

Thank you very much! I am

Perignon's picture

Thank you very much! I am in the processing of changing code. I do not want to highlight my project. I want to talk about the "elephant in the room" that no one else here wants to discuss.

Rules, regulations, polices, and coding standards are friggin awesome and I love the open source community for it as we are able to fix things faster than any cooperation can. Lets just apply the rules to EVERYONE. Not just new people. Because as already highlighted above, there are some "corner-stone" modules that would fail the current project review process.

Let us fix the problems. Not appease the people submitting new stuff (myself included). I will fix my code to be current with standards and policy. I reap what I sow!

Case in point. Go do a git clone of Views. The #1 installed, download, used (going into core of D8) module on the planet. It will fail a PAReview for the ludicrously simple reason that it still contains a "master" branch in the repo.

I very well know that the onslaught of project request are phenomenally unreal. But let us eat-our-own-dogfood. Do not apply to a new user/contributor to what you do not apply to some of the most influential and prominent projects that support Drupal. That is the very definition of being discriminatory and what WILL drive off new people.

Yep, both black

sreynen's picture

Would this module ever pass trials to be a project?

Yes, but not without fixing most of those errors first. Projects have problems after approval and we deal with those through issue queues. Anyone can open issues on projects, just like anyone can review full project applications. Both happen almost entirely with unpaid volunteers, which drastically limits our resources. So we focus those resources at the beginning of the process, in full project reviews, and hope that feedback will carry through. Sometimes it doesn't.

I totally agree it's not fair, but I think it's mostly unfair to the existing full project maintainers. As a maintainer of several full projects, I would love to have someone point out every bug they can find in my projects and help me figure out fixes. Unfortunately, the level of focused feedback people do in full project reviews is incredibly hard to find. I encourage you to enjoy it while you can, and if you're willing to spend your time finding more black kettles, seriously, please start with my projects.

You can run pareview yourself

fuzzy76's picture

You can run pareview yourself :)

This is exactly the reason

fuzzy76's picture

This is exactly the reason I've been nagging about replacing the review process (or atleast removing 90% of it) in favor of applying automated scores across the board for ALL Drupal modules. That way, code like that will be exposed on their project page. Both to warn users and shame the maintainers to fix it.

Last comment. Simply

Perignon's picture

Last comment. Simply put:

Why should I acknowledge the PAReview.sh review when other projects on Drupal fail it's review?

For the same reason you

tripper54's picture

For the same reason you should follow road rules even though some others break them.

Not exactly the same reason, you're not going to cause a road accident by leaving spaces at the ends of your lines, but you get my drift. Citing other people's bad behaviour is no excuse for behaving badly yourself.

If you find a module on contrib that's not following code style standards or has insufficient documentation, log an issue or even better submit a patch.

Do you really think forcibly removing views from drupal.org because they have a master branch in their git repo is a good idea?

Simply put: "For the same

Perignon's picture

Simply put: "For the same reason you should follow road rules even though some others break them." If you don't follow those road signs and an authority sees you. What will happen? You sir will get a ticket or fine.

Driving the same stretch of road for 50 years, if the local authorities decided to change the speed limit on that road and after 50 years you do not decide to change your speed to the new limit. You WILL get a ticket! It's posted as the kind officer will tell you when he is writing your citation or driving you to the ATM if you are in Europe.

Same here. The rules and regulations are posted. Drupal has coding standards and practices that thus far the community has agreed upon.

What I am trying to point out that the code review process is great and wonderful on new projects. BUT it needs to be implemented against current projects and Drupal Organization as a combined forces needs to have some leverage over the developers to change their code. Outside of the joe-smith submitting an "issue".

Honestly. What would happen if I submitted a "bug" to views that their git repository had a master branch? I would be told to go pound sand at the nearest beach. Hey... I am audacious enough to go do it! But I am just saying I would be pushed to the side.

Frankly ... to show that you

jthorson's picture

Frankly ... to show that you have at least an awareness of Drupal best practices, and a willingness to both co-operate and collaborate with others in the community.

There is only one gate, and once you're through this process, you have free reign; as do every other maintainer in the contrib space. We aren't going to babysit the coding standards of every module that everyone releases, but we do want to at least ensure each new contributor is equipped with the knowledge and tools required to be the best maintainer they can be, if they chose to be ... and that is the most we can do. It's up to the individual as to whether they actually put that knowledge to practice in the future.

As for forcing all projects to abide by the same standards we enforce in the PA queue ... if you're offering to go corral the wild horses, so that they can be run through that gate again, then may I wish you the best of luck!!!

Strangely enough for some

Perignon's picture

Strangely enough for some reason you cannot push Views to go through the current PAReview process.

why would you do that? The

klausi's picture

why would you do that? The Views maintainers have already proven that they are trustworthy and know what they are doing. Views is a huge module and keeping it up to date with current coding standards is not trivial - but I'm sure they would happily accept patches that improve the code style where possible.

Also: Minor coding standard issues are not application blockers, only if the violations exceed a certain volume we will ask you to fix them so that we can review the code better.

Why would I do that? Because

Perignon's picture

Why would I do that? Because it is being enforced here... In the program application process.

Yes the Views maintainers have proven themselves to be capable of writing good code. No doubt. Bad ass module! Does that mean they always do so? No... As proven by running a code review over the module. However, you must have a pure 100% clean report to be able to past muster in the project review process or raise-hell to get your project through - one or the other.

To your statement: "only if the violations exceed a certain volume we will ask you to fix them". But yet this rule doesn't apply to existing users that can promote any project to full project status? Again: "kettle call the pot black". Rules and regulations should be applied to ALL not a FEW.

Another thing I want to highlight is this setting an example and a process that people can pick up on. Get one module through and your are done - free reign. This is happening in the process of getting sandboxes to project status! I have noticed this behavior taking place over the last month that I been working on my module. Call it "black market trading" for lack of a better word. It is the other side of the fence that you as an approving official are not seeing.

Lastly, I think the process is stellar. And it's going to keep good code going out onto the site. BUT something needs to be done about the existing "crappy" code that - according to standards placed here - shouldn't be on drupal.org.

BUT something needs to be

jthorson's picture

BUT something needs to be done about the existing "crappy" code that - according to standards placed here - shouldn't be on drupal.org.

If that's really your message, then you're arguing it in the wrong place. Here's the Drupal.org Improvements group: https://groups.drupal.org/drupalorg ... best of luck. :)

My thinking on this is that a

iaha's picture

My thinking on this is that a guideline specifying a minimum number of lines will simply motivate inexperienced coders to write more lines of bad code in order to satisfy the quantity threshold. I don't think it will solve any problems.

Missing The Point

deaconblues's picture

I think the points raised by Perignon, and the resulting comments, bring up areas that need to be addressed. The statement of that once you prove that your module can pass PAReview process initially 100% and then you have free reign on check-ins after that will be an issue going forward. Once you drop the first cut of a module is when the real work begins. You have to have the same coding standards in place throughout the SDLC to ensure that you stay at a high level of quality. If not, developers will inevitably get sloppy as time goes on if there is nothing there to help self police the code base.

Not to harp on Views, because it is a great module and very beneficial, but it is a core piece on most Drupal sites and should be subjected to rigorous testing on an ongoing basis. That being said, I'm really surprised that you can't push Views through the PAReview process. I wonder why.

If that's true, I'd speculate

jthorson's picture

If that's true, I'd speculate it may be because PAReview was built specifically for this queue, and may have hardcoded drupal.org sandbox logic into the mix.

We have a similar problem with the testing infrastructure, which was built to test 'releases', and thus doesn't support sandboxes very well.

Nope it will run on a full

Perignon's picture

Nope it will run on a full project. I just can't get it to run on Views. Possibly the repo size could be an issue. It's huge.

See my link above for the PAreview.sh of Logintoboggan

Ever Expanding Circles

davidmac's picture

I had raised the point previously that the newest contriutors to Drupal are required to do the most reviews, if they wish to use the Bonus system to push forward their project. In some cases, individuals have been required to do some 12 reviews of other projects before their's is approved, because the bonus tag is removed after each review.

This, without any doubt, is contributing to the "fly by" cursory reviews that mention minor issues such as spelling mistakes and coding standards. Why, because new contributors, in many but not all cases, do not know their hooks from their elbow's when it comes to reviewing other peoples modules. Now, this is perfecty understandable and not a criticism, how can new contributors be experts? they can't.

This creates a frustrating process for those using the bonus system, and module contributors feel agrieved when their application issue is put back to "needs work" on the basis of these "fly by" non-technical reviews. This issue really needs addressing and I mention it again because it has been raised by others to some extent above.

On the flipside, I recently worked with someone who had submitted a module you could write on the back of a cigarette box, and I worked with them to double it's size and get it's code, and more importantly functionality, to an acceptable standard. I couldn't help feelling that she felt I was being pedantic and on several occasion she ignored important changes, with the end result being that the process became more frustrating for me than I had ever expected. It was difficult, really frustrating.

However the advice is to stick with projects as you review them and help
bring them to conclusion, which I did. This advice is entirely undermined by the 'review bonus' program which requires that applicants review multiple projects, leaving just one comment. Ther worse their own expertise is, the more reviews they are required to do. There should be a limit on the number of reviews required in these cases.

So there are two sides to the argument, as ever, but people raising objections at the process should firstly take a novice module and work with it until it gets approval. Only then will you see how difficult it is to properly review project applications (not just code, but installing them and really testing them).

The reviewers do a great job in difficult circumstances, and there is trust needed after someone achieves Git commit privileges.

My advice is, adhere to the coding standards, do a few decent reviews of other projects, and use the bonus system. You will actually be surprised at how quickly your module will be approved if you follow the steps.

Finally, a question, is there a group/comittee/plan of action for improving the approval process?

Note that there is also a lot

klausi's picture

Note that there is also a lot of positive feedback about the review bonus program, as applicants gain the important code review skill in many cases and can just pull themselves out of the waiting list.

Yes, there is a plan to improve the process: https://groups.drupal.org/node/291343

In theory there is a committee for the project application process, the drupal.org software working group: https://drupal.org/node/2001496 . But they are quite busy at the moment with the drupal.org D7 upgrade, and even for the near future only 3 new features (not related for the project application process) have been planned: https://association.drupal.org/node/18588

So I would not expect any big changes before 2015, unless someone starts a dedicated effort with the software working group.

It's Generally Positive

davidmac's picture

I agree, it was a postive experience for me, probably because I had researched all of the requirements thoroughly and really checked my code like a Squirrel with OCD, which not everyone does (although they should).

I would like to point out though that there seems to be a reluctance to discuss the imbalance in the review bonus sytem. As this is the second time I've mentioned the number of reviews required, and also given that other Drupalers have also mentioned the 'contradiction in terms' of requiring less experienced developers to do reviews, i.e. if their own project doesn't meet standards, they are required to do more reviews, as their issue is set back.

Sorry to mention it again, but I do feel that it is an unanswered topic of discussion, and I can't help stating that those who design the review bonus system should be open to discussing it in detail. For instance I have never seen a justification given for the fact that the review bonus tag is removed. Given the open nature of our community it is somewhat frustrating when issues only receive selective responses.

As far as I can tell, the different review bonus systems can be changed immediately, without committees or steering groups, as it appears to be at the discretion of those that have designed them, so I feel they should be open about the reasoning behind them and justify the reluctance to change them.

Just to reiterate my previous comment

Reviewers do a great job in difficult circumstances

... and the review bonus sytem can be really positive, but it could be improved if there is a will to do so.

I have mostly designed the

klausi's picture

I have mostly designed the review bonus system and I use it to prioritise applications that I want to review, because I cannot review all applications.

Please start a new discussion in this group as we are quite off-topic in this thread now. Original review bonus proposal: https://groups.drupal.org/node/203233

Previously mentioned

davidmac's picture

I had raised it 'on-topic' here: https://drupal.org/comment/7151296#comment-7151296

However, it wasn't addressed. My remarks are directed at the review bonus process, which I support, but I can't help feeling that this point, which has been raised by others is being batted away. Although you created the PARreview bonus program, it is seen as the main way to progress applications and therefore attracts a lot of discussion. Nobody expects a single person to do all reviews, that's why I help out when I can.

I am happy to join the other discussion, however I don't think we can limit individual discussion threads to a single aspect of a wider complex process.

I think this is a theoretical

klausi's picture

I think this is a theoretical problem, since it is very rare that applicants go through more than 3 rounds of review bonuses, especially since kscheirer is on duty as git admin.

I circle back to ex-review bonus applications quite often in recent times, because the review bonus list is empty very often.

So I don't think there is anything to discuss here, because it does not affect any current application nor in the foreseeable future.

Shutdown

davidmac's picture

Thanks for your reponse.

The examples are there, and they aren't theoretical. It's about changing the requirements.

With respect, and you deserve a lot of respect for the work you do, the level of frustration arising from the approval process is not helped by the fact that people are encouraged to join endless discussions and it seems that contentious remarks are summarily shutdown.

Well, we need to discuss to

klausi's picture

Well, we need to discuss to come to conclusions and decisions. Again, please post any points about improving the review bonus system as a new discussion, ideally with actionable items. I will gladly provide feedback and I am open to change.

I think it's very important

jthorson's picture

I think it's very important to keep one thing in mind:

The review bonus program is not mandatory ... and in truth, it's not even really official. It currently happens to be the most efficient way through the process, because it is how the most active volunteers prioritize their volunteer time. The only remarks that I see summarily shutdown are those which have already been brought up and discussed to death (and multiple times) ... it is not in anyone's best interests for those still actively reviewing applications to waste time further engaging in the circular dialogue.

My experience has been that, within the Drupal community, change comes into being because someone does something about it ... forgive the expression, but talk is cheap and action speaks volumes. Admittedly, many of those who come through this queue may not have the experience within the community to be able to negotiate a change into the process; but that's where others are here to help. The starting point, however, is with a defined action that you want to implement ... "I want to implement a code quality badge on project pages, how can we make that happen" will get you miles further than "The code quality on some projects suck, and someone else needs to do something about that!"

New issue

davidmac's picture

Well I have raised a new issue as requested here https://groups.drupal.org/node/374478

Becuase it's not mandatory, what does that mean?, 'we either like it or lump it?', 'if we don't like it, don't use it because we can't change it?'.

It's no good saying that something is unofficial, when it is so widely used, that's just bureacracy, which is one of the things that really frustrates newcomers to Drupal.org. The fact is that if it looks, feels and smells official and was created by officials, then it's official. Time to get the rubber stamp out.

The new issue I've posted suggests making it official and bring all reviewers into the bonus system.

I think this is what

dsnopek's picture

I think this is what @jthorson means by it's "not mandatory" and "not official": the reviews are done by volunteers in their free time. We can't require volunteers to review the applications with the bonus first. The reviewers are able to review which ever project they feel like.

I suppose it could be made "official" by preventing applications without the bonus from being reviewed or something like that. However, any review (even if the project doesn't have the bonus) is probably better than no review because the queue is always behind. :-)

Exactly right. And another

greggles's picture

Exactly right. And another side of being "not official" is that if someone disagrees with it then they are welcome to review applications in the order that they feel is most important. So, if someone feels that the review bonus system is not good they should take the action of doing reviews of applications that don't have a bonus (or do, but had it removed, or whatever their prioritization is).

OK

davidmac's picture

OK, perhaps he meant that. But what I meant by making it official, is to encourage other reviewers to remove the review bonus tag when an issue needs work, and spread it's use because as I understand it, it's generally Klausi who removes the tag given that he created it for his own workload.

It's just an idea, but I think it could be pushed more to the forefront and we would start to see the benefits of its increased prominence and hopefully uptake.

Add one note to the review

Perignon's picture

Add one note to the review bonus process. You mentioned the "fly by" reviews and you can see that easily within the queue.

Another behavior that it has started is the "you review my module, I'll review your module" black market trading I was referring to.

Backing up a level, I am not exactly targeting just the review process in my comments. I'm looking at the problem at large, the amount of poopy code we have on D.O. I know at a recent drupal camp (Atlanta) this was a topic of one of the speakers when discussing D8. Maybe change can happen sooner than we think. D8 may be the catalyst for this change. But with it has to come the policies to manage the entire project life cycle.

The Technical Working Group

jthorson's picture

The Technical Working Group also has the 'Git Vetted' process fairly high on their radar.

Should be very high on the

Perignon's picture

Should be very high on the radar because from my observations as a person seeking to become a project contributor it is a broken process and coming from my background (see below). It isn't performing the function that it was designed to do.

Case in point: I cannot promote a sandbox/project to full status; however, I can release code on D.O within an existing full project - I am doing it now!

Loopholes exist in the process. I believe fixing the loopholes is the wrong way to approach it, that's sticking fingers in a leaking levy. What should be fixed is the total Software Development Life Cycle (SDLC for buzzword bingo) of projects and their release onto D.O.

I was the project lead and chief engineer over a software development project that had a yearly budget of $10million a year just for development costs. I helped build a community around our core software where we started to build an open source project taking contributions from users/developers. The software is used on highly secure networks, we had to fix this problem and it was fixed by putting in place a vetting process for submitted code (yes all of it). We were able to automate a lot of the review process using a cobbled together set of tools like Jenkins, Jira, and some other well known packages. It can be done! I know personally that there are some commercial companies that will give their products and services for free when it is for high profile projects - they did for us. Commerce Guys has this processes for the modules they develop for Drupal Commerce. They work with the payment companies to vet their code. This is a great process and I am so very, very happy they are doing this. It's making Drupal Commerce a secure and trustworthy platform. Maybe the Technical Working Group should pull in the Commerce Guys into this problem.

I am eating my own dog food

Perignon's picture

I was given rights on a project about two months ago and I am in the process of fixing the code to standards! :) https://drupal.org/project/edgecast

A little research on this

jthorson's picture

A little research on this site will show you that I'm well aware of the issues, and have been involved in trying to change them for a very long time. Long enough to realize that you can't boil the ocean, and things need to be broken down into bite-size actionable steps if you want anything to move forward. I've been involved in the 'this process is broken, and needs to be fixed' debate more times than I care to count, and was responsible for raising it myself way back around before DrupalCon London (just to learn it had already happened at least twice before).

The fact is, discussion at this 100,000 foot level gets us nowhere, frustrates those who have been through it before, and takes valuable volunteer resource time away from actually implementing the types of things you're hinting at.

See https://groups.drupal.org/node/291343, see https://drupal.org/node/2034437, and see https://drupal.org/project/issues/1367220?categories=All ... my advice? Pick a piece, and make it happen.

As for the TWG, it's still in the formation stages ... for more information there, check out https://drupal.org/governance/technical-working-group.