New recruits and splitting up reviews

Events happening in the community are now at Drupal community events on www.drupal.org.
ccardea's picture

There has been some discussion recently about modifying the process to allow for less experienced people to get involved in the code review process. I've been thinking about this and so I thought I would set out what I believe are the pitfalls of this approach.

First of all we have to realize that there are different types of organizations. Some, like production or assembly type operations can get the job done with relatively less skilled workers, and only need a few more highly skilled workers to manage the operation. In other types of environments, the bulk of the work gets done by highly skilled professionals. In those types of organizations, the lesser skilled workers usually take on the role of administrative support, and they are usually fewer in number than the skilled workers who do the bulk of the work. Obviously the latter situation applies to the code review process. In principle, I'm not opposed to having someone screen project applications for module duplication, licensing, and maybe some very basic documentation checks, but there should only be one or two people who do this on a regular basis, not a whole cadre of new recruits.

A good reason not to split up code reviews is that it makes the process more difficult to manage. You need to be able to track what stage the review is in, so that people who feel qualified for that stage can find them.

Another question is where else could the process be split? The one place that comes to mind is there could be a separation between code review and security review. But that doesn't really fly because anyone who can review code should also be able to review security practices. Then again, module duplication is an important issue, and do we really want to leave that to administrative support personnel? So where does that leave us?

We need people who can actually do the technical review. So where are we going to find them? A recent suggestion was to have a Code Review Week. That might work if we can find a way to publicize it. I think our most fertile recruiting ground would actually be the Project Application queue itself. A lot of these people have a pretty good understanding of Drupal and coding standards and security and documentation. They also have a vested interest in helping to reduce the backlog. So I think that as part of the process, we should check new applications immediately to see that they have good project descriptions and links to the sandbox, then INVITE THESE PEOPLE TO VISIT THE CODE REVIEW GROUP, and maybe join up and participate in the process.

Comments

Here's Another Thought

ccardea's picture

The backlog of applications is now as long as six weeks. Something drastic needs to be done or new contributors will definitely go elsewhere. I think we could go as far as to make contributing to the process a condition of getting git vetted user status. In other words, if a project has reached RTBC status, we say to them, in order for your application to be approved, you need to complete x amount of code reviews. It's the only leverage we have.

I speak as someone new to

dreamleaf's picture

I speak as someone new to contributing in this way, but it would seem wrong to say to people "thanks for you contribution, but it's only getting approved if you do x,y,z on top of the actual review process as well".

There are also varying levels competence of contributors too, ranging from those needing lots of coaching to the established (know drupal well) people. It would be wrong in my mind to put them all in the same pot and by the same rules. The process of having a review is just that, having a review... not being made to contribute time, energy etc outside what that initial contribution allows.

I do think it's a great idea to make polite requests for people in the queue to start looking over other submissions, but I do think one of the reasons people don't do it currently is that they are "new" in one way or another and without the acceptance that they actually do know their stuff, are in fear of looking a prat for commenting something that may be incorrect.

I also think that the more hands on deck looking for issues such as duplication the better, I've personally informed much longer serving contributors of modules that are in existence that they weren't aware of. In my experience a lot of longer serving members have a specific core knowledge and they stick pretty much within that remit, so they may have a general overview of the landscape, whereas newer people tend to come with a particular ideal in mind and have therefore researched (stumbled upon) a completely different scope of resources.

Difficult as it may be, tagging or checklisting the process is the robust way to go about it. Then people of all levels and those waiting in the queue can see a need and get involved. Currently there is not a way to gauge what has been done and what hasn't. For me, I'm more than happy to go through and check for duplication, licence checks and some formatting issues, but I wouldn't dream of commenting on the more technical based reviews as that requires a deeper all-round knowledge than I currently have.

It's not wrong, it's negotiation

ccardea's picture

We have something that they want. If they want it enough, they will do what we ask, and if they don't, Drupal will be fine without their contribution. In the real world, this is how things get done. These are also people who have been vetted, so that solves the problem of asking someone who is too inexperienced to do code reviews.

As for the module duplication issue, your point is well taken. It might cut down the backlog a little by weeding out the duplicators.

I'd respectively disagree ...

jthorson's picture

I'd respectively disagree ... If the mission of this process is to 'promote lasting, long-term contribution' to Drupal, then encouraging contribution needs to be at the forefront of every decision.

The 'exclusivity' angle works for Apple, but does not work for open-source. If you put up barriers, people won't contribute. I believe this is a well-established consensus within the Drupal community ... we need to get rid of the current barriers, not implement new ones.

Contributors have something 'we' want - not the other way around. Your approach assumes that those who contributes code crave acceptance and will go to any length to get it. I'd argue that the vast majority build code to solve their own problems, and then apply to give back to the community ... and if the community doesn't want it, no big deal. It's not the contributor who loses out in this case.

I completely agree

sreynen's picture

I completely agree with jthorson here. Drupal is a volunteer community. Requiring anyone to contribute their time isn't really an option.

Respectively?

ccardea's picture

It looks I'm alone on this one, so I'm not going to try to push it, but seriously, why do so many people want to contribute to Drupal? It's not out of the goodness of their heart. They want something they can put on their resume, a feather for their cap. A lot of them really want the git vetted user status so that they can publish all of their projects. And we have a serious problem that needs to get solved. We need to use the little bit of leverage that we have until the situation improves. It doesn't have to be as heavy handed as flat out telling somebody they're not going to be approved unless they contribute. That actually would be a corruption of the process. But we probably could do something like expedite the applications of people who participate.

Despite some people having

dreamleaf's picture

Despite some people having motivations like you think, I would disagree that the main motivation is a resume filler. I think a larger motivation is that people (me included) have spent a long time learning Drupal, hanging around the community, and dabbling in participation. And then the day comes when they do:

a) have something to give back
b) feel confident enough to stand with the big boys, girls and others

Bearing in mind a lot of contributors are making money somewhere along the line by using Drupal, one could assume that they are seeing their contribution to Drupal as an extension of that investment of time and expertise - one thing they aren't getting just by getting git vetted is status :D

Two Stage Process

ccardea's picture

Your scenario is a two stage process, initial screening and technical review. If we modified the workflow a little, we could do that without tagging. We could have the initial status set to active, instead of needs review. That would signal that the application needs screening. When screening is done it could be set to needs work or needs review. That would allow screeners to search for active status and reviewers to search for needs review.

Let's try this

sreynen's picture

This seems like a very workable short-term improvement. We need to get most reviewers on board before this will work, though, so we can safely assume "needs review" means the "active" part of the review is complete.

Flip the status?

jthorson's picture

The only issue with this is that the documentation (and all of the current applications) currently direct users to set the 'needs review' flag themselves. This was a major motivation for suggesting the tag route.

However, this might be something that could be implemented immediately without needing to communicate the change (via documentation and to users) if we flipped the meaning of the flags ... using the 'needs review' status to indicate the 'screening' part of the review is NOT complete, and the 'active' flag to indicate that it's been screened and is now ready for 'technical' review.

I'd rather stick to the original plan

ccardea's picture

1 'needs review' means the application needs to be reviewed. Making it opposite would likely create confusion.

2 My plan is to go through the entire queue and mark everything that hasn't been touched yet 'active', everything else stays the way it is. I can explain the change of status when I make the change. At the same time I plan to invite the applicant to join the group and do some reviews.

3 I'm only aware of one place in the documentation that discusses how to set the status in the application queue. That will be easy enough to change.

Once you've started the

jthorson's picture

Once you've started the "screening" stage, you've gone from "needing review" to "review has been started" ... in other words, "active". This approach doesn't require updating every issue in the queue, and works within the current structure.

Touching every issue in the queue will result in every issue having the same 'last updated' date, which makes it impossible to pick off the oldest applications first. If you were concerned about someone being able to 'game the system', this accomplishes the same undesireable result.

I'd rather stick to the original plan

ccardea's picture

The undesirable result is that we continue to argue over small details, while the backlog in the application queue keeps getting longer. I am presenting a solid plan that includes a sustained effort to recruit new people to the code review process, gives them an incentive to do so, and provides an easy way to manage it.

The real reason for going through the queue is to contact applicants about participating in the process. If I start at the back of the queue, the oldest applications will still be the oldest applications with status 'active'. New applications will continue to be marked 'needs review' until the queue has been updated.

Any more problems?

Sounds familiar ...

jthorson's picture

...we continue to argue over small details

I had a discussion with zzolo in IRC yesterday, and to paraphrase his comments (correct me if I'm wrong, zzolo!), we have a number of discussion threads with some suggestions and ideas; but we have no formalized means of making decisions within the group.

Shrinking the queue is a noble goal (and one I share), but how do we evaluate any given proposal on how to go about doing that? How do we communicate changes out to applicants and reviewers? How do we evaluate the effectiveness of any given change after the fact? How do we determine priorities between various initiatives? How do we go about developing group (and community) consensus?

With this in mind, the impression I got is that zzolo would prefer to start with the basic process-building for the group itself, rather than moving forward with any drastic changes to the review process itself.

On another note, I'd ask (beg!) you to not go through and touch every issue in the queue, at least until I've had a chance to work out http://drupal.org/node/1179586 and get it implemented on drupal.org.

More inaction

ccardea's picture

This strikes me as being like hurricane Katrina and Joplin, Missouri. We have a significant problem on our hands, but instead of acting to solve the problem, people want to talk about formalized decision making processes, and writing new software. We have group notifications, and we have polls that we can use to communicate with the members. Since the goals of the plan are to reduce the backlog by bringing in new people to do reviews, it is very easy to measure the effectiveness. There is (or should be) only one top priority, and that is reducing the wait time for code reviews. The time for talking is drawing to a close. We need to get to work on solving this problem. I'm sorry but if I feel like I've got enough support for this, I'm not going to wait for new software to be implemented.

Choose your horse and flog it ...

jthorson's picture

And piss on anyone else who tries to do the same ...

... going full bore ... And you're pushing your own agenda when you should be getting on board with the existing leadership.

Sound familiar?

Well? Which is it going to be? Get on board with existing leadership ... or pick an agenda and push it down our throats, slamming anyone who tries to promote a slightly different alternative?

As per your email:

Your Agenda is:

1. You want to bring in new inexperienced people to do code reviews.
2. You want for them to do partial reviews.
3. You want the review process to be your process.

Well, right back at you. Sorry if I feel you're being a bit hypocritical here. Then again ... I guess maybe pissing on people's ideas, rewording them, and then re-pitching them as your own is your view of progress.

/sarcasm off

My policy is not to respond

ccardea's picture

I'm not here to engage in this kind of thing.

Here for what? To Collaborate?

jthorson's picture

Your harsh criticism of my post, flat out denial of my request to hold off for an existing initiative, refusal to entertain suggested 'tweaks', and desire to push forward with a plan which has been flagged '-1' by at least three different group members would suggest otherwise.

"This kind of thing" is no different than what you've done in other posts. Demonstrate some flexibility, objectivity, and openness to other ideas, and you'll find that this courtesy is paid back in return. But respect is something that comes with demonstrated integrity ... and arguing against a suggestion in one thread, then proposing the same suggestion yourself in another (or going around marking legitimate tweaks as '-1' simply because of who posted them) are not very good ways to try and rebuild that respect once it's been lost.

I don't appreciate false accusations

ccardea's picture

I was talking about a two stage process before you even arrived on the scene. http://groups.drupal.org/node/143594 I stated above what I don't like about using multiple people for reviews, but I went along with the group on this one, because that's what everybody seems to want to do. Not only did I go along with it, I came up with a way to manage it, that I think will work better than tags. To my knowledge that hasn't been suggested anywhere else.

Now I suggest that you stop embarrassing yourself with what has the appearance of childish temper tantrums. I didn't like your little tweak. Grow up. Deal with it.

Blind to your own actions.

jthorson's picture

I thought your policy was 'not to respond'? :p

Go reread http://groups.drupal.org/node/151909 ... Remember, this whole thing started with your overreaction (temper tantrum?) to having your wiki page modifications reversed.

I'm simply requesting for consideration, respect, and collaboration. Your tone over the last three weeks has granted none of the above. Comments like "Get your priorities straight", "childish temper tantrums", "You're part of the problem", "you're pushing your own agenda ... get on board with leadership", "Respectively?", "Any other problems?", "More inaction", "Grow up, Deal with it", and "This wiki is rejected"; along with sideways accusations that I'm trying to "game the system" and "pushing some hidden agenda" do nothing to foster a friendly and cooperative environment.

None of the above is productive ... THIS is why you get the reaction you do.

Noone else has an issue with me, and I don't have issues with anyone else. Go read http://drupal.org/dcoc and consider it's ideals when forming your comments ... and you'll quickly find that not only am I easy to work with and open to alternatives, but my suggestions are backed up by both research and experience, and often make a LOT of sense if you're willing to grant them the slightest consideration.

This is an ugly game that you're playing

ccardea's picture

The game here seems to be, go along with jthorson, or be subjected to a lot of personal attacks and abuse. I'm sorry but I'm not going along with a bad idea just to appease someone. That's called mediocrity.

What we seem to have here is someone who sees every criticism or rejection of an idea as a personal attack on him, and so he responds in kind. Possibly also a little 'Not Invented Here Syndrome' thrown in as well. This behavior is really shameful. I think you need professional help.

.

jthorson's picture

That's EXACTLY what's going on ... you've just got the name wrong.

In any case, the 'game' switched over to one called "Troll the troll" a few posts ago. Phrases like "I think you need professional help" only encourage additional rounds.

I'm not posting this because

dreamleaf's picture

I'm not posting this because I know jthorson or particularly agree with his points (although I do), I would just like to comment that I've been following all the threads and it would appear from a 3rd party perspective that @ccardea doesn't appear to favour criticism or others disagreeing with his points.

We all have our own thoughts and potential solutions to the problems, but in a community such as Drupal we all have to enter into discussions with a level of maturity that says "people WILL disagree" and "I may not win, but I WILL go with the majority viewpoint".

As a willing peacekeeper, I would strongly suggest that all bad feelings be left behind and an understanding be remembered that we are all here for the same reason, and indeed, playing for the same team. There is nothing more destructive to community harmony (and bringing new people into that community) than seeing less than polite exchanges as we have witnessed here.

So, let's all have one big virtual hug and agree to disagree.

(can I have a badge that says Peacekeeper?)

Check the link

ccardea's picture

I urge anyone reading this to follow jthorson's link to 'game the system'. There's absolutely no suggestion that jthorson was connected in any way. What does that tell you? jthorson, this has to stop. You're going to get both of us booted.

.

jthorson's picture

Agreed, it has to stop.

And all it takes is a single post that doesn't include a sideways dig, or even a half-hearted attempt at respect.

Admittedly, I got sucked down into firing back ... but only after trying keeping things 'above the board' for a very long time did I switch over to the 'tough love' approach.

Should have asked earlier probably

mermentau's picture

I know I should have asked earlier, but it would be nice if we could expose the Drupal major version that the project is for. Some only feel qualified to review one version and now you have to go to the git viewer most of the time and look at the .info file. Then later if you are going through you forget and do it again. I know there are some projects that I have checked 3 times for the version.

Ideally the applicant would show that prominently in the project description, but it really doesn't get done nearly enough.

Drupal major version

ccardea's picture

I will make a note in the code process draft. This is something we can check for in pre-screening.

I don't want to wait too long

ccardea's picture

I think we need to act fast, but we do need to make sure that people are aware. I will take responsibility for changing the documentation. I believe I can do a group notification. And I will start sending out the invitations to join the group. I think just a few more days should be enough time to give others a chance to weigh in.

Code review for security advisory coverage applications

Group organizers

Group notifications

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

Hot content this week