This issue has been finally moved to a documentation page: http://drupal.org/node/1975228
Read on there!

Files: 
CommentFileSizeAuthor
#27 visual_design_0.png501.12 KBzymphonies.com

Comments

barneytech’s picture

When I click and edit my project it does not let me add any tags Klausi. This is my application url: http://drupal.org/node/1328366/edit. I did list the reviews I've done though.

klausi’s picture

Tags cannot be added in the issue summary, you need to do it via a comment.

klausi’s picture

Issue summary: View changes

Updated issue summary.

klausi’s picture

Issue summary: View changes

Updated issue summary.

vaibhavjain’s picture

I am very much confused over the review of a module and to get this tag.

When is a review of module considered to be reviewed? when we are done with manual review only or even helping and reminding them about the code standards, branching or telling them how to apply for a full project is considered as a project review ?

should we add the links of the project when we tell others to follow the guidelines, or should we add the link when we do a manual review only ?

Highly confused, please clarify.

klausi’s picture

A good review contains everything: hints to coding standards and other guidelines that are not followed by the project, but also the results of you going through the source code of the module (that is what I call a manual review).

anilbhatt’s picture

@klausi,

i am agree with #3 it is bit confusing , how can i get a review bonus, even i have done 3 reviews. Please add some more examples.

I Have done 3 reviews and after that i applied for bonus, and you added your comment and removed tag please see it http://drupal.org/node/1416240#comment-5557214, it would be good for all of us to understand it properly.

anilbhatt’s picture

Issue summary: View changes

changed needs review link to list oldest issues first.

klausi’s picture

I added some example applications to the issue summary above.

After you did at least 3 reviews of other applications you have to reference them in the issue summary of your application issue. You can edit the first post (=issue summary) of an issue. Create a new section "Reviews of other projects" and add links to the exact review comment, e.g. like this one http://drupal.org/node/1381726#comment-5418942

patrickd’s picture

After you did at least 3 reviews of other applications you have to reference them in the issue summary of your application issue.

It's all in text, please read more carefully and ask more precise questions.

patrickd’s picture

Issue summary: View changes

added example applications

klausi’s picture

Issue summary: View changes

fixed example link

klausi’s picture

Issue summary: View changes

added review bonus list for review

ethanw’s picture

This process is a great idea, as someone who currently has a project in the queue and has worked with others to help get projects approved, I'm very excited to see a system of rewards put into place.

I'm a bit confused about how the requirement to review 3 applications between each PAReview: Bonus filing relates to the "How to review Full Project Application" page's guideline to "Attempt to stay with a single project application". I read that line as meaning one should only review a single project at a time, however with applications often taking around 6-8 weeks, this could mean 2 months or more in between each PAReview eligibility, depending on the responsiveness of the module coder, etc.

Am I misunderstanding that line?

Or are we moving to a more distributed system, where reviewers can contribute to two or three reviews in a batch and help move them along, then apply for the PAReview status?

Personally, the idea that latter, more distributed system, sounds like a good way to go. But I've only been part of a handful of projects, so can't claim any higher level insights into what works best.

Thanks for all your hard work!

klausi’s picture

I would interpret it like this: if you have done a review and the applicant has fixed the issues you should review that application again. In the meantime you can review of course other project applications, if the applicant does not respond soon enough.

However, I don't think that this is fair. We have 150+ open applications right now which are unanswered for up to 4 weeks. So I personally look at the oldest applications needing review and I distribute my review effort. The only exception I want to make are applications that take part in this review bonus program, because I want to reward those that help others.

Maybe we should remove that recommendation to stay with one application from the documentation.

vaibhavjain’s picture

@klausi
Infact we can change the way it is said.
You have explained it in a much better way, this can be a part of documentation too. :)

nitvirus’s picture

Does this applies for themes also??

jthorson’s picture

nitvirus,

I'm pretty certain it applies for all 'project applications', regardless of the component type (module, theme, etc.).

jthorson’s picture

Issue summary: View changes

fixed pareview.sh link

klausi’s picture

Issue summary: View changes

Improved approved issues link

klausi’s picture

Issue summary: View changes

oldest issues should be reviewed first

klausi’s picture

Issue summary: View changes

More introduction text to explain the necessity of this.

patrickd’s picture

Issue summary: View changes

Adding some useful resources

muhammad.tanweer’s picture

Project: Drupal.org security advisory coverage applications » AutoMobile
Component: other » Code
Assigned: Unassigned » muhammad.tanweer
Status: Active » Needs review

I need a review of this theme which I wrote for Drupal 7. Its in sanbox as of now. I am not completely clear on the process of getting a project from sandbox to full project. Any help would be highly appreciated.

Thanks,
Muhammad

patrickd’s picture

Assigned: muhammad.tanweer » Unassigned
Status: Needs review » Active

Please read https://drupal.org/node/1011698, https://drupal.org/node/894256 and its subpages.
If you still got questions you can contact me directly, don't use this meta issue.

patrickd’s picture

Project: AutoMobile » Drupal.org security advisory coverage applications
Component: Code » other
Cristian.Andrei’s picture

Hi,

Can you please update the instructions from the "How to get on the high priority review list" section to be more clear as, right now, it is not clear if the reviews need to be manual code review (as pointed out by klausi in this comment) or some other form of review.

Thank you,
Cristian

Cristian.Andrei’s picture

Issue summary: View changes

Made "manual review" bold

patrickd’s picture

Issue summary: View changes

made 'manual' even more fat

patrickd’s picture

Issue summary: View changes

added pachecklist

klausi’s picture

Replaced "review" with "manual review" where applicable. BTW: everyone can edit the issue summary, so you can fix it yourself where you think it is unclear.

klausi’s picture

Issue summary: View changes

added "manual review" even more often

mitchell’s picture

I would like to see, "We need your help to manage the flood of applications in order to get a review for your particular application" improved.

That message is disconcerting and it encourages meaningless interactions in the issue queue (crappy reviews), which is wasteful of our most precious resource.. people. Or to be more specific, people helping people.

I think we should lower the number to "1 or 2", and massively encourage or raise the stakes for those people to stay involved with that specific review(s), so as to become peer-mentors with/of/by/for/to each other. Let's use this tool to help people make friends and become good collaborators. Besides, most of the reviews of early contributors aren't very helpful to mods anyways, so at least they might be useful for helping people meet one another and having someone to ask questions to/from.

I think this implies breaking out the multiple parameters of PAReview Bonus into multiple tags, making some significant docs updates, and also figuring a better way to distribute the workload. Maybe there are other changes that would help with this.

klausi’s picture

yes, there are bad reviews sometimes, but there are also very good reviews. And applicants learn a lot while looking at other people's code, I heard that often from them. Crappy reviews are better than no response in 8 weeks.

I don't think we should lower the number, we desperately need that help. I have never been a fan of "stay with one application" because it is a bit unfair to the other applications that don't get reviews at all.

And of course those reviews are helpful to administrators, at least all issues raised by automated tools are cleared out before I even look at the project. Let's use this tool to keep the number of open applications down.

Not sure what you mean by breaking out multiple parameters?

I think we should continue the discussion in the code review group and leave this issue for more specific questions/comments to how review bonus currently works. http://groups.drupal.org/code-review

a_thakur’s picture

Yes, reviewing other code is a great learning experience. A bad or brief review is anyhow better than no activity, as in case of no activity the thread creator sometimes feels disheartened.

webchick’s picture

February:

"We have 150+ open applications right now which are unanswered for up to 4 weeks."

July:

We currently have 107 "needs review" applications with oldest awaiting reply at 7 weeks 6 days, so I'd say the overall situation is getting worse with this new process. :(

On the brighter side, it looks like the PAReview: review bonus queue is actually moving along pretty well (10 needs review, oldest reply at 2 days, 18 hours).

However, there's nothing about the existence of this tag, nor the existence of this process on http://drupal.org/project/projectapplications. There is passing mention of it in various handbook pages, but it's really easy to miss within huge walls of text, and it links to this issue in the issue queue rather than an actual piece of documentation.

I'd recommend editing the project page and various docs to let people know that they are required to review other peoples' apps to get their applications reviewed and move the process on how to do so to more "official" documentation. Since, regardless if this process has actually been formally agreed to or not by the code review team, it's clearly what reflects actual reality.

klausi’s picture

Unfortunately we lost many regular project application reviewers in recent months that have moved on to other things. greggles is now security team lead, tim.plunkett works for Views in core etc. So I think it is wrong to draw the conclusion that review bonus has made the situation worse. Quite the opposite: review bonus keeps this queue alive even with this low number of reviewers.

+1 for mentioning review bonus on the project page of this issue queue.

+1 for moving the issue summary to a documentation page. I did it as issue in the beginning to collect comments and to give support on questions that might arise.

I'm not so sure about making review bonus required/mandatory. We already had that discussion and I agree that it might scare people away or whatever. There are still project applications that get approved without review bonus from the other Git admins. It is just me that refuses to take a look at applications without it ;-)

webchick’s picture

You're right, that was out of turn. It's clear that this new process has made certain applications fly through the queues, and that's great to see that at least some of the problems of the past are being partly addressed.

However, the queue wait times really speak for themselves. Regardless of what the outcome was of that discussion, and regardless if this adds yet another barrier to an already high set of barriers for getting involved in the community, it's pretty clear to me as an "outsider" that the only way to get someone to look at your project is by reviewing others'. IMO we should just be explicit about that fact, as it'll likely cut down on the number of angry emails from people feeling ignored, despite following the documented process.

I also think that we need a Plan C (or maybe it's Plan M by now :)) that accounts for another fact, which is that we don't, never have, and never will have enough people capable of doing the depth of reviews needed to support our current heavy-weight processes, even despite the fantastic work that's gone into automation recently.

patrickd’s picture

We're already steadily pointing out that "there are about 100 other applications waiting for a review and only a hand full of reviewers. We highly recommend you to get a review bonus". (Dunno how more obvious we can get without directly telling them that they MUST do a review, what is and should not be the case).

Yes either we get more people in here to review applications or we have to make this process more lightweight. But how lightweight can it become by still checking for duplication, security and licensing? I don't think here's much room left between "go through through this (endless-)one-time-approval-process" and "click that button and do whatever you want".

klausi’s picture

Added a hint to review bonus to the projectapplications project page.

klausi’s picture

Issue summary: View changes

manual review = read source code

zenlan’s picture

Oops, meant to post this on the other thread.

zymphonies.com’s picture

Project: Drupal.org security advisory coverage applications » Business-Enterprise-Sandbox
Component: other » Code
Status: Active » Needs review
FileSize
501.12 KB

I need a review of this Business-Enterprise Drupal 7 theme. please review

http://drupal.org/node/1791592

http://ventral.org/pareview/httpgitdrupalorgsandboxshanidkv1781600git

Thanks
Shanid kv

klausi’s picture

Project: Business-Enterprise-Sandbox » Drupal.org security advisory coverage applications
Component: Code » other
Status: Needs review » Active

Restoring status.

Please don't move this issue around. We need you to review other project applications, please read the issue summary.

zymphonies.com’s picture

ok, sorry,

i have to review others projects.

ramsonkr’s picture

Now add an issue tag to your application: "PAReview: review bonus" (this can only be done via a comment).

Highly confused, please clarify.

klausi’s picture

On the comment form there is a "Tags" fieldset above the save button. Open it and fill in the issue tag.

zymphonies.com’s picture

i have reviewed three projects. and added those projects in issue summary.
so can i add "PAReview: review bonus" tag
https://drupal.org/node/1791592

klausi’s picture

@shanidkv: sure, but those reviews must be manual reviews, as I indicated in #1791592-15: Business-Enterprise. And please use a permalink to the exact comment of yours, so that I can easily verify them.

fuzzy76’s picture

So I have spent some time ranting, some time digging around and now I will try to be a bit more constructive than I have been earlier. :) I feel that some consequences of the review bonus system is disruptive to the quality of the reviews. I'm not saying the bonus in itself is a bad idea, but I think it has several weaknesses that needs to be adressed somehow.

How to review Full Project applications says

Attempt to stay with a single application. Given your time, it is beneficial to stay with a single application (issue ticket) from start to finish. This provides consistency to the applicant and better mechanism for the feedback loop.

But in the queues you will often see people jump in, give a review to get their bonus, and then leave without ever providing feedback on the fixes. Over and over again. My favorite example is Deep Survey which had 10 (TEN!) different reviewers. This causes a lot of disruption for the project applicants, as the feedback in the reviews are all over the place, as well as causing them to wait for a new reviewer to take over each time they submit an update. This makes the entire process feel chaotic for the people involved, and quite frankly gives a terrible first impression of our community.

Drupal.org project application workflow says

A 'needs work' status change should be reserved for 'major' code quality issues. Minor changes, recommendations for full API compatability, and txt file perfection are not major code quality issues. Holding up reviews on these changes can come off as unnecessary and nitpicky, especially early in the review process. This can also cause frustratingly long waits for uninitiated applicants.

This is not how the queue actually works today. People are getting "needs work" from reviewers for issues that are nothing more than "not best practice" kind of stuff. I suspect a lot of people looking for review bonuses will be overly nitpicking in order to "justify" their reviews, but I'm only guessing here. It is a very real problem though.

thelee’s picture

I'm confused about this whole process, as mentioned in comment #34.

I'm trying to see what applications I can look at it to get a review bonus for my own project, but I don't really see "fresh" projects that I can review from start to finish, as per the docs. Instead, at most I see projects that have already had reviewers that pop in. What qualifies as a good enough manual review to warrant referencing in my project application?

More theoretically, isn't the "three reviews for one review bonus" ratio dooming the vast majority of people to never getting a review bonus (since strictly speaking only 1/4 applicants will be able to get a review bonus since for everyone one project that gets a review bonus, another three projects get reviewed) OR dooming a lot of applications from getting a lot of redundant reviews just so that people can try to fast-track their own?

jthorson’s picture

@fuzzy76:

I understand your criticism, and when viewed in light of the 'ideal state' (i.e. how the process is 'supposed' to work), it is completely accurate and valid. Unfortunately, the process has never approached this ideal state ... with this in mind, it helps to compare the process now to where it was, say, 18 months ago.

The "try to stick with a single review" comment is a leftover from the initial implementation. In practice, applicants often take weeks between corrections, based on their own priorities and other work impacts. This makes it very difficult to maintain any sort of flow when working with a single application. While it was a noble goal, the size of the backlog has forced us into a mode of 'help as many as you can whenever you can'; and the 'stick with one application' recommendation has fallen by the wayside.

The current state has many people joining the thread, leaving their two cents, and then disappearing again. I would agree that a large percentage of these comments are not particularily helpful, and can lead to frustration for the applicant. However, well far from ideal, this is much better than the equivalent state about 18 months ago; where instead of the 'noise', applicants would get zero feedback for many weeks on end. Some applications could be two months old before a reviewer picked it up ... and, finding it hard to read due to inconsistent formatting, they would raise the 'coding standards' flag and move on. These days, it may still be two months before deep-dive reviewers pick up an application; but by then, the 'little things' have been worked out. The Review Bonus program has helped us trade silence for too much noise ... neither is perfect, but the current state is the lesser of the two evils.

On the 'needs work' front, this is also an artifact of the initial program implementation. The design was that the 'needs work' flag would be set every time a reviewer found an issue or made a suggestion for improvement. In theory, it makes sense; and ensures that applicants recieve and/or address any comments provided by reviewers. The flaw in the design is that it did not anticipate the long turnaround times between deep dive reviews; where setting something to 'needs work' essentially meant creating a new 4-week delay for that applicant (even if the fix only took 10 minutes). As a result, many of us have stopped bumping things to 'needs work' for more trivial items and suggestions for improvement; but this is only a fairly recent development ... we have months of precedence to overcome before the documentation change will fully filter down into the implementation itself.

thelee’s picture

@jthorson

Further question - what exactly happens to reviews that fail to get review bonus? I'm looking at one right now, and honestly I can't find any area for further refinement that doesn't amount to insanely tedious nitpicking ("maybe you should camel case here"). So I could leave a useless manual review of "looks fine" but klausi on the thread himself has mentioned that s/he won't review applications that don't have a review bonus so I'm not even sure if there would be a point to adding a "looks fine" note.

jthorson’s picture

@thelee,

When the Review Bonus system was first implemented, we found a large number of participants would simply copy a repository link into the PAReview site, paste the results into an application thread, and then call that a 'review'. The whole 'manual review' terminology came about to discourage this practice, which does not provide a lot of value to the end applicant ... essentially, a 'manual review' means that you're actually taking enough time to look at the code, and possibly installing it and testing that it works (simplytest.me makes this trivial). Don't worry about finding a fresh application and sticking with it from start to finish ... any application that isn't already 'fixed' can benefit from testing and/or additional pair of eyes.

In my mind, even if you don't feel comfortable doing a full security audit on a project or reviewing someone elses code ... taking the time to install the code, verify the module does what it says it does without throwing any errors, and adding a comment saying "I installed the module and spent 10 minutes testing it, and found that the functionality all works without throwing any errors or unexpected watchdog logs" is valuable enough to reference it as a manual review. That said, I don't use the review bonus program to prioritize my time, so your milage may vary. :)

On your second point, the three reviews to one bonus ratio is not as unbalanced as you infer, since the review process is iterative and the majority of applications take more than three iterations before they are promoted. The '10 different reviewers' that fuzzy76 references above is actually very common.

klausi’s picture

"Staying with one application" is really outdated and does not fit our current workflow, I removed that form the documentation: http://drupal.org/node/894256

I agree with jthorson: it is fine to just post a note that says that you reviewed the code and tested the project and found no flaws, that is a valid manual review to me (don't forget to set the RTBC status, too!).

Unfortunately a lot of people just ignore the review bonus program, that is why the ratio of reviewing 3 other applications does not bring down the number of open applications to zero. It still helps a lot though :-)

In return I take the liberty to ignore non review bonus applications, so we have some angry complaints from time to time. I can only repeat: we are a community and we help each other, I'm counting on you!

webchick’s picture

In return I take the liberty to ignore non review bonus applications, so we have some angry complaints from time to time. I can only repeat: we are a community and we help each other, I'm counting on you!

And I can only repeat: this has got to be more transparent to people (i.e. not buried in a 40+ reply issue in a ~300-issue queue) because it's absolutely valid for people to file angry complaints when they attempt to be one of the 0.05% of people out there giving something back to Drupal and get ignored for their efforts.

webchick’s picture

Incidentally, the Technical Working Group is a governance body we're trying to set up in order to manage policies/processes of this nature in the future. People involved in this thread may have thoughts about that.

klausi’s picture

@webchick: and I also repeat this one more time: almost every application gets a comment from me like this:

"We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)"

So it is transparent in 98% in the 300 issues you mentioned. It is also transparent in the documentation: "It is strongly recommended that you take part in the review bonus program, otherwise you will not get reviews of your code for several weeks. So make sure to review three other project applications before posting your own." http://drupal.org/node/1011698

I don't think we can do more for transparency, do you have any suggestions?

webchick’s picture

Yes. Change:

"Review bonus
Applicants can speed up their application by taking part in the review bonus program."

To:

"WARNING: Your contributions will be ignored unless you participate in the review bonus program."

Or something similarly explicit.

thelee’s picture

Thanks for some clarifying responses. Quick question though @klausi:

"I agree with jthorson: it is fine to just post a note that says that you reviewed the code and tested the project and found no flaws, that is a valid manual review to me (don't forget to set the RTBC status, too!)."

What's RTBC?

klausi’s picture

@webchick: Thanks, fixed the sentence on the project page to "Warning: applicants should take part in the review bonus program, otherwise their applications are likely going to be ignored." and in the docs: http://drupal.org/node/1011698

webchick’s picture

Cool. That's extremely rude and disrespectful, but at least it's telling people the truth. :) Thanks.

klausi’s picture

@thelee: RTBC = reviewed & tested by the community.

@webchick: why do you think it is rude and disrespectful and how would you improve it? (I have the feeling that you you find the whole review bonus program rude and disrespectful, which is BTW not at all felt by our applicants as we found out in our survey last summer.)

I think I'm going to add "We are a community and we help each other, so we are counting on you!"

webchick’s picture

Sorry. I meant the fact that we ignore people who give back to Drupal is rude and disrespectful. But it's even worse to do this and not warn people ahead of time, so they can make an informed choice about whether they want to make an investment in our community.

Frank Ralf’s picture

Thanks for the input.

JFTR linking this to two related postings: "Module Approval Process is Too Slow" and (my own) "Buried in the sand(box)".

fuzzy76’s picture

I notice that both my issues regarding differences between the documented review process and the actual process were "leftovers" of dead documentation. This should tell us that the documentation of the process is broken, which in turn means the applicants and the reviewers have no idea what's in store for them. There should definitely be a clearer document that describes all aspects of the process instead of wiki-pages and book pages spread all over the place as it is now. Only then can we actually discuss what's wrong with the process.

thelee says:

I'm trying to see what applications I can look at it to get a review bonus for my own project, but I don't really see "fresh" projects that I can review from start to finish, as per the docs. Instead, at most I see projects that have already had reviewers that pop in. What qualifies as a good enough manual review to warrant referencing in my project application?

This is exactly one of the problems. A lot of people probably feels like this, and "puffs up" their reviews needlessly. To avoid this we need a much better documentation of what should be regarded "needs work" and what should be left as a suggestion but not a blocker. And that "worked fine, don't see any issues" actually IS a review as long as you did a minimum of testing. Though it would be easy to fake, as there is no requirement on the people who do the actual reviews.

jthorson says:

The current state has many people joining the thread, leaving their two cents, and then disappearing again. [...] This is much better than the equivalent state about 18 months ago; where instead of the 'noise', applicants would get zero feedback for many weeks on end. Some applications could be two months old before a reviewer picked it up [...] These days, it may still be two months before deep-dive reviewers pick up an application; but by then, the 'little things' have been worked out.

Actually, if an applicant does not have time to participate in the Bonus Review Program, his project will not get approved. Ever. So we changed a lot of projects from "two months" to "never". That is favoring an arbitrary group (people that have a lot to do in their job or hates tedious testing) against another (those who do not).

webchick says:

I meant the fact that we ignore people who give back to Drupal is rude and disrespectful. But it's even worse to do this and not warn people ahead of time, so they can make an informed choice about whether they want to make an investment in our community.

This is exactly how I feel about how the Review Bonus Program is applied. Turning down (what can turn out to be) extremely good developers because they don't want to test out strangers' projects that are completely irrelevant to them is rude, disrespectful and damaging to the Drupal community.

People might even have spent a lot of time convincing their bosses about publishing these modules. And then find out they need to convince the boss to give them time to do completely unrelated volunteer work before the module is published? By making the bar of contribution this high, you are alienating a lot of people which might end up keeping their great module for themselves instead.

The Review Bonus Program is fine as a bonus. As a mandatory requirement it should be renamed to "Review Requirement". And then removed. :-/

We do have 20.000 modules, vetting all new ones using volunteer work is not scalable. That is the problem that needs to be solved. This feels more like solving the problem of "we are getting to many applications" by making the bar of entry higher.

Slightly off-topic: I really don't know why there is no black market for comaintainer positions yet. Or if there is, I want in. It's a clear hole in the system than can probably get most people in without showing a single code-line. And some of us richer. ;)

davidmac’s picture

Would it be fair to set a limit on the number of reviews a contributer needs to do in order to keep the review bonus tag?

At least one review I have looked at recently required 4 rounds of reviews to be done, as his issue was set back to 'NW' several times. In total he posted 15 links to reviews (although he only required 12). Where is the limit and should there be one?

Looking at this from the perspective of the other projects being reviewed, does the intermittant removal of the Pareview tag create a situation where less experienced developers are required to do more reviews than the ones who get their module code largely correct in the first instance, and could therefore be considered better placed to review other projects.

In addition, it is unlikely the review guidance of "sticking with a project/issue" can be sustained if the number of reviews rises to these levels.

I think it is appropriate to set a limit of say maximum 6 or 9 reviews required per project issue, after which the review bonus tag would remain until completion.

klausi’s picture

Yeah, the documentation is a bit fragmented right now and could be unified, although it will certainly not fit onto one doc page. You could move over some stuff from groups.drupal.org, move this review bonus issue finally to a doc page etc. Just ping me if I should review any changes you did!

We still get applications approved that never had a review bonus, you just need to check the fixed applications to verify that. The review bonus system works so well that some code review administrators lean back and are not that active anymore. I'm currently trying to engage with them more often to keep them involved.

Note that this is a one-time approval process, so of course we do not have to check every new module on drupal.org.

fuzzy76’s picture

When a nearly 2 year old application still can sit as RTBC for 2 months without anything happening (until I created some noise), I doubt there are that many others without review bonuses that get through. One every second month is "close to nil" in my book. Hence the documentation change by klausi in #45 to reflect that the Bonus Review Program has become a de facto requirement.

And I actually think the part about "one-time approval" is a bad thing on its own, but that's probably more fit for another thread. The rules applied to the applicants are constantly broken by people "on the inside" with no repercussions. I have several modules I depend on - with code I'm actually ashamed to have running on my server, but not enough time to fix them (and getting maintainers to accept code cleanups from strangers is pretty futile in a lot of cases).

As a long term solution (probably also off-topic) I think a right balance of automated tests and user ratings can be applied equally to new and existing modules, and provide everyone with better metrics on the modules they use. And this will scale too infinity.

sreynen’s picture

For what it's worth, I'm against saying "otherwise their applications are likely going to be ignored." That's misleading. They're likely to be ignored by people only looking at review bonus tags, but that's not everyone. Every issue will be reviewed eventually. Not using the review bonus system likely makes your review go slower, and I'd much prefer saying that to the the "ignored" terminology, which wrongly implies that it will just sit there forever.

jthorson’s picture

@klausi:

In return I take the liberty to ignore non review bonus applications, so we have some angry complaints from time to time.

I don't presume to suggest how you prioritize your volunteer time ... but while the review bonus program has helped limit the number of applications that get posted and never looked at, having RTBC applications actively ignored and siting idle for months is (IMO) even more insulting to applicants than ignoring their initial application. I beg you to reconsider your policy, and relax it in the case of the RTBC queue ... what good is all the additional activity your program has created if we still stall folks out at the last gate?

@webchick: Thanks, fixed the sentence on the project page ...

First priority is incenting new contributions ... we can't allow ourselves to lose this focus while incenting their participation in reviewing other applications. The phrase "...your application is likely to be ignored" is presumptuous and elitist ... which in turn encourages less participation in the overall application process, and also the overall community (by driving them to Github or another alternative).

I've reworded both pages.

jthorson’s picture

@fuzzy76:

This should tell us that the documentation of the process is broken, which in turn means the applicants and the reviewers have no idea what's in store for them.

... the Bonus Review Program has become a de facto requirement.

This is one of the reasonse that the 'Technical Working Group' webchick mentions is being created ... as a community, we have a tendency towards single users being able to implement widespread policy change through the act of editing a single document page, or 'this is how I do it' becoming 'this is how everyone should do it'. What you're seeing is the result of multiple years of frustration, and numerous disjointed attempts to try and fix it.

... without anything happening (until I created some noise),

Bold, self-congratulatory, and presumptuous. I could counter that nothing happened until I had two days off work and was able to look at the queue for the first time in a while ... I prioritize by 'longest idle' and 'created date'. I also only get back to the queue about once a month, if that. :/

Actually, if an applicant does not have time to participate in the Bonus Review Program, his project will not get approved. Ever. So we changed a lot of projects from "two months" to "never". That is favoring an arbitrary group (people that have a lot to do in their job or hates tedious testing) against another (those who do not).

Now this is pure FUD ... it discounts the work being done by reviewers who do NOT use the review bonus program to prioritize their efforts, myself included. I would be somewhat insulted; only I recognize that this is the precise reason that we have so few active Git Admins still participating in the process. As I mentioned above, there is a couple of years worth of community frustration associated with this process. Every few months, someone stirs up the the discussion again; citing how hopelessly broken everything is ... but seldom are there any actually actionable suggestions as to how to fix things, and never is there any recognition for those who are still plugging away, trying to improve the situation. Over time, the emotional baggage associated with this cycle wears down on people; to the point where they finally give up and move on to other areas of the community (where their efforts are more appreciated).

The review bonus program is *NOT* mandatory. If, in practice, it seems like it is, then that is due to the fact that we do not have enough volunteers to maintain the queue; but this is not klausi's fault. Throwing the Review Bonus program under the axe is not going to solve things.

I normally wouldn't call someone out like this, but it's obvious you feel passionately about the topic ... so please ask yourself "What am *I* doing to improve the situation?". If your answer to that question is simply "raise awareness of the issues", then I would ask you to stop ... we're already intimately familiar with the shortfalls, and are more than a little tired of beating that dead horse. What we really need are more change proposals, with actual actionable solutions.

mitchell’s picture

What if, as a means to get the RB tag, users could participate in another project's issue queue, or participate in documenting another project, or do a sufficient amount of 1-on-1 mentoring, etc etc?

Maybe in this way project applications will be more tightly integrated across d.o and a wider audience will gain awareness of users' applications. I think the main benefit is that it gives users an incentive to be creative and deliberate about how they are to get involved in the community with something they can continue to build on after their application is completed, and these are skills that could come into use while maintaining their own project.

fuzzy76’s picture

I know I might come off as a bit harsh and might use more pointed arguments than necessary from time to time, and apologize if I offend anyone.

Don't think I hate the bonus system as it is defined. After all, it does give more eyes on each project and probably help all the applicants learn more in the process. But all the bonus programs in the world can't make enough volunteer hours to keep up with the growth of Drupal, and this should have been evident for years. Those folks "plugging away" are doing a good job, but they will always barely be able to keep their head above water. I want to help them (or obsoleting them if need be).

A lot of my despair over this comes from the discussion over at Module Approval Process is Too Slow, which was the first large discussion I read about the subject. And the reason for my despair is that the thread contains a lot of very good suggestions, and they were all ignored.

The Technical Working Group seems to be a MUCH needed thing. As far as I can tell there haven't been a real "owner" of this process earlier, so it seems like nobody has been able to (or dared?) make any real decisions over a rehaul. Hopefully, they can look at all the existing suggestions and find viable options.

So @jthorson, when you say that there are seldom actionable suggestions how to fix thing, I strongly disagree. There are plenty, but somehow they never come to fruition.

When it comes to my "FUD" arguments, all I am saying is that 2 months wait for that last change when the module actually seems approved, might as well be "never" from the applicants view. And that, in turn, makes the Review Bonus Program much closer to mandatory than optional in my book (after all, there's probably a scale between optional and mandatory in all our heads).

My motivation might have started with raising awareness. But most people near the center of our community know about this issue by now. But that makes it even more frustrating to see a lot of suggestions be met with total silence. And in some cases, totally wrong angles (like how to get more volunteers, which won't scale). I want the process reviewed, and hopefully the TWG can make that happen.

sreynen’s picture

When it comes to my "FUD" arguments, all I am saying is that 2 months wait for that last change when the module actually seems approved, might as well be "never" from the applicants view. And that, in turn, makes the Review Bonus Program much closer to mandatory than optional in my book (after all, there's probably a scale between optional and mandatory in all our heads).

I disagree. That would be true if review bonuses were the only way to get reviews, but they're not. I've experimented with two different approaches that both had at least moderate success: posting in groups on groups.drupal.org related to the focus of your project, and blog posts on Drupal planet. This probably won't scale, but I have so far written a blog post about every application that has asked for one and they've all been reviewed within a week. I don't have as specific data on posting to related groups on Drupal Groups, but I'd estimate around 80% of those posts get reviews within a week.

More generally, the most troubling thing for me about discussions like this is that most of us are talking as if we have no control over this process, which is not at all the case. Those of us who don't care for the review bonuses can and do prioritize reviews differently and try different strategies to get more people doing reviews. And this works. Changing what others care about (for example, trying to get klausi to stop caring about review bonuses), on the other hand, has a proven record of failure. And beyond failure, it has a record of making people give up and just leave.

It's also not true that anything in the "too slow" thread has been ignored. I've read every comment there, and I can't imagine I'm the only one. I haven't chosen to act on every suggestion there, but no one needs me or anyone else to act on their ideas for them. We can and should act on our own interests.

webchick’s picture

I don't know the backstory/history with fuzzy76, but I will say that his comments precisely echo dozens or probably hundreds by now unpleasant e-mails/conversations/rants/etc. I've been subject to when I attend various Drupal events, go on-site at companies who use Drupal, etc. Just so this doesn't give off the impression of being one lone person's opinion.

Thanks for the additional tweaks to the project page's description, jthorson.

jthorson’s picture

fuzzy76,

If you truly want to help, where's your "How can I help?" post? I'm sure many people would be happy to take you up on the offer and point you in the right direction.

Klausi saw issues, and created the review bonus program to try and address some of them, which (despite it's shortcomings) revolutionalized the process. He also spent a ton of time on Drupal CodeSniffer, and integration with Coder, which brings us much closer to completing the holy grail fo full process automation. Patrick_d created http://simplytest.me, which will again revolutionize things. This is the type of 'plugging away' that I'm talking about ... and it is this type of work that gets interrupted when we find ourselves responding to the never-ending stream of complaints on g.d.o or in the issue queue.

If someone were to approach me, I'd point them at http://drupal.org/sandbox/jthorson/1367220 ... a framework and some examples on how to interact with the projectapplications queue. Enhance away.

Or there is http://drupal.org/project/securitytesting and http://drupal.org/project/security_scanner, both which could be a start towards automated security scans upon module creation.

Then there is http://drupal.org/sandbox/jthorson/1779754, which is an attempt to build an interface on drupal.org, so that the required automation has a place to live. Since the drupal.org port to D7 didn't happen, it needs to be backported to D6 before it can be implemented. Feel free to take this on (though I've been told the vertical tabs needs to go for accessibility reasons).

There's also http://drupal.org/project/drupalorg_qa and http://drupal.org/project/drupalorg_qa_worker, which were supposed to be the next-generation job schedulers for drupal.org. Once again, we need the drupal.org D7 port, which didn't happen. But if it had, http://drupal.org/sandbox/jthorson/1805836 and http://drupal.org/sandbox/jthorson/1686406 were ready and waiting in the wings, to automate the PAReview and Git repository portions of the review, fully integrated with drupal.org.

Half of my blog posts on planet are related to the projectapplications process. I've got two more half-written ... One is a future state vision of what the project applications queue could evolve into ... to give people a target "end game" to build towards, instead of simply tweaking here and there without any overall direction. The other is a "Clear the projectapps queue by [date]" call to action would have been posted yesterday, except that i) I didn't want to compete with Drupal Sprint Weekend, and ii) I spent 5 hours in the projectapps RTBC queue yesterday instead of completing the post.

Why aren't any of my sandboxes above implemented? Well, I have a non-Drupal day job, and spend an additional 10-20 hours a week maintaining Drupal's quality assurance automated testing infrastructure on the side ... a role I inherited while researching PIFT/PIFR as an option for automating the projectapps queue; only to discover that that noone was giving that system any love, either.

So forgive me if I get a little riled up with someone comes along and suggests that nothing is being done, and all these excellent suggestions are being met with total silence.

You say you want the process reviewed. To what end? A review alone simply tells us what we already know ... the process is broken. The TWG will not be able to help here unless they have another alternative to evaluate against the current status quo ... one that still takes into consideration protection against spammers and namespace squatting, helps contribute towards a high standard of code quality within Drupal contrib, and does not simply throw the gates wide open and overburden the security team with a flood of insecure module (as they include any contrib module with an official release within their mandate).

If you have ideas for a solution which addresses the above, please share them ... I'd be happy to drop everything else I'm doing right now, and start coding the solution tonight; if someone could just tell me exactly what it is.

fuzzy76’s picture

sreynen and jthorson, your arguments about "everyone can make a change" is only true for small extras like the bonus program. I can't invent and implement a new process for drupal.org on my own, and quite frankly, I should not be able to. This would require consensus.

jthorson, my "how can I help" post has been missing because I've seen people try to get new processes discussed, a lot of people agreed, and then nothing DID happen from the perspective of the discussions. There were no conclusions to be found. It seemed like everyone was waiting for someone with authority to say "yes, let's do this".

I see now that you have been working on a lot of the stuff that have been proposed, but I haven't seen anywhere in the discussions a clear conclusion about going that way. And personally I would not have started developing a lot of "stuff" without knowing wether my work would get implemented in the end or not. Or is there a post/conclusion somewhere I've missed?

But since I've derailed this poor issue enough already (I started my complaint here about how frustrating the demand of review bonuses on RTBC issues seemed for applicants), I'll try to end on a more productive note...

JThorson, would you be willing to do a rehash of your old post at "Meta Discussion: Project Application Process Revamp" (which I only discovered yesterday, and which I think is awesome!) so we can get a "fresh" discussion on it and submit it to the TWG as a proposal on their first day "on the job"? Or if the TWG proposal fizzles, let's riot for it. :) In order to have it easier to implement right away, the automated testing can be cut to what's ready, the review might be on today's level and the auto-pruning could be put on hold until the automated testing has been expanded enough. It would still filter out a lot from the issue queue and be a much better foundation to build on.

webchick’s picture

@fuzzy76: Not sure if there's any way you can make the trek from Norway to Portland, but the call for core conversations is still open for DrupalCon Portland, and would be a great place to propose such a talk. And it'd be great coming from someone new, rather than the same old people who always talk about this. ;) If not Portland, then perhaps Prague.

I think the key piece you and lots of other people are missing here with:

It seemed like everyone was waiting for someone with authority to say "yes, let's do this".

...is that no one has authority on this currently. (That's what the TWG is attempting to codify—thoughts about that over in that governance issue would be really welcome.) However, all the people who are perceived to have authority (not only in this but in all other areas of the project) generally have it simply because they saw something was wrong and rolled up their sleeves to do something about it. That's it. That's their magical superpower that blesses them with the ability to make change—they simply form the project towards the outcome they'd like to see. :) 45 seconds of insane courage and a few free afternoons/evenings, and you could be one of them. :)

This is why you're seeing pushback from people who are currently "doing" in this portion of the "do-ocracy." They feel strongly that if fewer people just complained about the state of things and instead just did what they did: write a quick module, take an afternoon off and sort through the queue, etc. that we'd all be whole a lot happier.

There's definitely some truth to that, and I'd encourage you to "scratch your own itch" as well, so you have a stronger voice in this process. It doesn't change the fact that the process we have is demonstrably unsustainable, but it would get us by until the TWG is formed and empowered to start making real change in this area.

jthorson’s picture

Thanks, webchick ... for once again suscinctly stating what I wasn't able in my own over-verbose way. :)

The frustration I experience with the complaints is exactly that ... they fall on deaf ears, because there is no authority to actually hear them. Part of the motivation behind the TWG is to pick up the slack in areas such as this, but establishing governance (by it's nature) is a slow process. The TWG isn't likely to get up and running before Portland at the earliest ... which is why I feel it would be better to have ideas (or even a fully thought out proposal) to put in front of them at the first meeting, instead of leaving it to the working group to come up with their own.

As for a revamp of that blog post, that has been in the works for about 6 weeks now. Unfortunately, I don't have a target date for when it'll be published ... It's next on my priority list after clearing out the projectapps RTBC queue (if the other Git Admins could pick that up, it would certainly help along my progress on a new proposal!)

sreynen’s picture

However, all the people who are perceived to have authority (not only in this but in all other areas of the project) generally have it simply because they saw something was wrong and rolled up their sleeves to do something about it.

Yes! The upside to no one being in a position to say yes is there's also no one in a position to say no. I've seen this work, and I wish I could get more people to believe it.

I first started doing reviews back when we were reviewing for CVS access, and one person was doing nearly all of the reviews. That person was as much "in charge" of this process as anyone has been. I didn't like how they were doing reviews. So I disagreed on a few specific issues, and I also started doing reviews myself. And I found that doing the work myself was way more effective than trying to change how someone else did it. Saying "I think you're wrong" after someone else says no was a week-long discussion. Saying "yes" before they had a chance to say no, on the other hand, completely circumvented that discussion. As long as my "yes" could be reasonably defended, it wasn't worth challenging.

For a brief time, I was doing most of the reviews, and I was effectively "in charge" of this process. And then the exact same thing happened again. Other people (including klausi and jthorson) saw better ways to manage this process than what I was doing. If they ever suggested I do things differently, I don't remember it now. What I do remember is they started doing things differently. And pretty quickly, they were "in charge" of the process.

Anyone else, including you, could just as easily be "in charge" of this process. It's a lot of work, but volunteering in your own concept of the right way is so much easier than convincing others they're volunteering in the wrong way. So much easier.

Frank Ralf’s picture

@#57

I agree and think that's what the Prairie Initiative's Reputation/badges system for drupal.org: proposal is all about.

fuzzy76’s picture

Good to see we can all end up on the same page here! :)

@webchick you are right, Portland is a bit far. I hope it will be discussed in Portland, though. I am convinced that we should have the solution clear (if not implemented) before Prague. :)

I think a do-ocracy has it shortcomings when it comes to wide-reaching decisions like this (I don't think a single person should be able to change documented processes that affect a lot of people), and those shortcomings are exactly what the upcoming workgroups intend to solve. So hopefully, we already have the solution. We just need to assemble and apply it. :)

@jthorson Great to hear that it haven't been abandoned! I'll try to help out, but spare time is a rare resource in my life atm. But I will definitely participate in any further discussions on it.

greggles’s picture

I haven't read this whole thread. I just want to say that I don't think the TWG is a silver bullet so I caution folks from postponing or pinning too much on its formation. There are problems in the project application process and no clear solution(s). The TWG can help in a few small cases like when there's a clear solution but an individual is blocking it.

From what I have read, I agree with comment #65.

Diogenes’s picture

@fuzzy76 - Thank you for having the courage to carry the torch on this issue and refusing to let it die. There has been plenty of debate, and many good suggestions on how it might be improved.

I think of the current process as random drive-by code reviews. The majority of people doing reviews are those looking for a review bonus to get their own module promoted. Many of them are new to Drupal and lack experience. They often just skim over previous reviews and the responses. Alternate API functions that have been tried without success are suggested again. Then there is the "duplication" bullet. It's like telling someone who has built a new fuel efficient car that what they really should be doing is to help make that '75 Cadillac over there more fuel efficient. That's the Drupal way!

If your module is reviewed and you respond with a reasoned defense, don't be surprised if you never hear back from the reviewer who fired that last bullet. They have already moved on and it's far easier to say nothing than admit that they may have been wrong.

Here is another take from webchick

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

And let's set aside the myth that "You can make a difference. Take an afternoon off, roll up your sleeves and just do it". That is exactly what happened with the creation of DrupalAnswers. It has the support and endorsement of many senior members of the Drupal community and has been the subject of one of the longest discussion threads on this site. Now try to find a link or any reference to DrupalAnswers on the DO website. It looks as if some top level decisions have been made by people who have fallen in love with their own code. The status quo thus prevails.

The best remedy, suggested by fuzzy76 (#53), mlcn and others, is to make every module subject to review. This might be a tough sell because the standards now are too high to get much buy-in. I'm not the only one who gets frustrated by pareview and coder. Pointing out that there are 2 spaces before the brace instead of 1, that a comment lacks proper punctuation or that there are 83 characters on a line rather than 80 is just plain annoying. It simply slows down the whole process with very little benefit. It's time to lighten up a little!

Since the migration of DO from 6 to 7 has stalled and failed (a huge black mark against Drupal), it might be a good time to start a fresh D7 site from scratch dedicated to contributed modules, coding standards and best practices. Implement up/down voting, comments and some kind of reputation ranking like the (gasp) StackOverflow model. Indicate which modules have passed review and which ones still need work or may have security concerns, but give all modules equal exposure, like the Wordpress model, so users can decide for themselves.

On a closing note, the discussion thread called "Module Approval Process is Too Slow" was originally called "Module Approval Process Will Kill Drupal". All one has to do is look at the last comment on Fuzzy76's favorite case study to decide which is the better title. chris_p displayed remarkable patience and professionalism. I really doubt he will try it again.

Diogenes’s picture

Issue summary: View changes

only major flows are blockers

klausi’s picture

Issue summary: View changes

clarified issue tags

klausi’s picture

Status: Active » Closed (works as designed)

This issue has now been finally moved to a doc page: http://drupal.org/node/1975228

If you would like to further discuss this please do so in the code review group: http://groups.drupal.org/code-review

Thanks!

klausi’s picture

Issue summary: View changes

move to doc page