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
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
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
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..
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.
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
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
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
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
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
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
@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
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
<?phpif ($condition_a && $condition_b || ($condition_c && ($condition_d || $condition_e || $condition_f && ($condition_g || $condition_h) || $condition_i)))
{}
?>
count versus
<?phpif (
$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
Thankfully we have coding standards, so your point is moot.
As out point out, the coding
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
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
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
@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
@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
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
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
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.
@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
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.
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
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
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
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
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
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
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?
certifiedtorock.com/u/36762/ | Druplicon debit card
Baffeled by The Application Process
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
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:
Thank You For The Detailed Answer
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
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
Thanks for clarifying everything. It is very much appreciated.
Unapproved developers doing reviews
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.