Posted by jordojuice on June 10, 2011 at 4:46am
Im still a newby and would love some feedback on my post on a recent project application. I began a review of the application at http://drupal.org/node/1183226 and found that the applicant admitted that his module was a duplicate of a D6 module. He mentioned trying to contact the maintainer of that module unsuccessfully. Did I handle it right in my response to the applicant by suggesting that he apply for maintainership of the project and return to the project application queue once it is received? My response was formatted around a similar issue I had seen recently. I just want to make sure I am not giving inaccurate guidance to our applicants.
(see the post for more information)
Comments
Yes, that seems right to me.
Yes, that seems right to me.
knaddison blog | Morris Animal Foundation
This was a new application
I know it's off topic but why are you looking at brand new applications when we have applications that are 6 -7 weeks old?
You can come up with ideas
You can come up with ideas for how to best review the queue, but responding to someone like this will discourage them. I appreciate the work you are doing, but I feel like you are trying a bit too hard to tell other people what to do which will frustrate and discourage volunteers. Having a reviewer help out who makes some mistakes is better than no reviewer at all.
Consider an alternate way of stating your opinion:
Which one of these responses is more likely to encourage someone to stay involved?
knaddison blog | Morris Animal Foundation
You're right
Technically, I asked a question, but it was a rather gruff question. I'm a little busy and don't always take the time to be nice. Sorry jordojuice, you're doing a good job and we appreciate it. Thank you and keep up the good work.
No biggy!
I am not discouraged and, indeed, it was a valid question to ask. Admittedly, I have probably reviewed some applications when others might have been more justified as they had been active longer, but normally that is because I try to take on applications that I feel comfortable with. However, my intentions in this case were different. I visited the top of the queue to briefly check the new applications in order to try to resolve any obvious issues with the applications. In this case, the applicant clearly stated that there was a duplication issue, so considering the backlog I felt it could be beneficial to swiftly resolve it by replying now, thus preventing the applicant from unnecessarily having to wait weeks before discovering that he was in the wrong place and still reducing the backlog.
I think there is something to be said for doing this as in some cases it can resolve issues that otherwise might have taken weeks of waiting before the applicant is notified that his or her module submission cannot be accepted. For another example, around the same time I responded to this issue I also found that another applicant had submitted two modules; I instructed him to close one of the applications (http://drupal.org/node/1180740). Taking action in this way I felt was as much a help to the backlog as reviewing an older application. Had the two applications continued through review process unnoticed it would have only been an additional and unneeded burden on the current backlog.
In summation, I feel that these types of issues can and should be resolved quickly. There is no sense in imposing a wait on an applicant whose application will ultimately not be reviewed when he could have otherwise been notified of a better course of action sooner rather than later. I can assure you I am conscious of the backlog as much as anyone. My own module has been in the queue for six weeks with no activity for a while, but I'm truly in no rush as I've found solace in joining this group and feel that the things I'm learning can only be beneficial to my work (in fact, I would prefer that we reduce the rest of the backlog over reviewing my module at this point). Ultimately, my intention was not to delve into the code review of a new application, but just to do a quick screening of applications that have apparent duplication or other screening issues. As for code reviews, for now I will be sticking with the applications in which I am already involved, and in the future I will certainly work to review the oldest applications. I intend to continuing scanning new applications. However, if I should not attempt to screen new applications for "deal breakers" then please tell me and I will refrain from that in the future.
It's an important first step
If you look at the new draft of the code review process http://groups.drupal.org/node/152274, 'pre-screening' is included as part of the workflow. This is an important part of the process, and I intended to do this myself, because I want to make sure that certain things get done, like informing applicants of the wait-time for a review and inviting them to participate in the process.
designated person
The draft calls for a 'designated person' to do the pre-screening. If you want to be the 'designated person', I think that would be ok, but it has to be done consistently and regularly, and you need to be fully aware of everything that needs to happen at that stage.
Why a designated person? That
Why a designated person? That sounds like an unnecessary bottleneck.
Agreed ...
I think the draft needs to reflect that 'anyone can take on any stage of a review' that they feel confident doing, and that use of the status flags/tags/whatever would only be necessary for 'partial' reviews.
Why a designated person
For the same reason that you have a designated person as the quarterback on a football team. I'm trying to establish a little bit of organization, so that everybody is on the same page and there's not a lot of needless duplication of effort. I should have said 'a designated person' instead of 'the designated person'. There's no need to limit it to a single individual, but I want to avoid too many hands in the pot, since we know that spoils the broth.
I'm assuming this screening
I'm assuming this screening is things like 'check that there's a sandbox link', and 'Sandbox page contains a project description'.
If any step in the 'pre-screening' step requires specialized knowledge, such that it can't be done by 'anyone', then the step doesn't belong there. Let the quarterbacks focus on the game, not whether everyone remebered to bring their helmet. ;)
This is not a football team.
This is not a football team. There are/will be lots of people participating, but without guaranteed schedules or hours committed. I don't see why it wouldn't be encouraged for anyone to participate at any stage of the process, so long as they are capable. The process needs to be very fluid. Having people designated for certain tasks (without specific technical reasons, like git admins) is likely to increase the review time, not decrease it, while people wait for designated individuals to show up and complete specific tasks.
That's what I thought might
That's what I thought might be the issue as well. Allowing multiple people to participate in any step of the process can help mitigate unforeseen problems like this. And closing off certain stages of the process could prevent contributors from helping when they otherwise could have. For example, if one is not skilled enough to do a code review but understands the basic requirements of the application process enough to provide a pre-screening, then why shut him out of the process in favor of a designated reviewer? Good thought!
Also because
Also because we don't need a lot of people working the front of the queue. We need to focus most of our effort on applications that have been waiting for a while.
.
We need people wherever they are able and willing to help. Not everyone can do code reviews but everyone can check for apps that are missing required bits or are duds in other ways. I wouldn't discourage anyone from helping, even if they aren't helping in the area that's needed most.
Michelle
Agreed
The applications that have been waiting the longest are largely those that are most difficult to review. I think encouraging new people to start at the easier end is a better way to ease them into the process. Also, the beginning of the queue becomes the end over time, so adding reviewers to the beginning now will shrink the end eventually.
Picking Projects to review
Some people will have areas of experience that make it highly time efficient for them to pick certain projects from the queue regardless of when they were put into the process. Myself I have only picked D7 modules because that is all I have any real experience with. I got interested in Drupal when the early D7 alphas started.
Being able to put my toe in keeps the bar low
I understand that in many ways it's fairer to the submitters to work from the oldest part of the queue first. But I'm much more efficient if I stay within my realm of experience: I've mostly worked in Drupal 6, for example, so I don't feel qualified to review a D7 application. Also I've been working lately in integration with third-party tools, so it's more efficient for me to look at those. And I am frankly intimidated by the size of some of these modules; I don't yet feel qualified to judge some of them.
As a newbie to the code review process I really like that I can take an issue from the queue that is best for me while still helping the group as a whole.
It's been the practice
There was somebody who had been doing this. And yes, it's not a football team, but some semblance of teamwork would be a positive thing. Chaos is not a positive thing, so I'm staying firm on this.
Just an observation ...
Just an observation ... "Staying firm" has actually caused 98% of recent chaos within this group ...
Hmm. First, to be clear, I'm
Hmm. First, to be clear, I'm not offended or anything and I aim only to improve the process. But this sounds to me a bit like saying
This (admittedly loaded) statement, to me, seems counter-productive and a bit anti-collaborative (awesome word! Is it real? I wonder...). But then again if it is settled that there is a designated pre-screener then I shodnt be looking at new applications! That being said, we've been encouraging new applicants with varying skill levels to review other projects in the queue (obviously that's not the issue). But are we going to lay down the rules to every new reviewer that comes in and begins pre-screening applications?
Considering this thought, I think @jthorson's observance could be relevant here and in some cases adding rules may indeed only cause conflict. Rest assured, I completely understand, respect, and endorse the efforts at organization. Similar to other areas of drupal.org, the project application review group needs some sort of structure, that is for sure. But we don't want to build too many walls.
.
I seriously doubt there's any such rule. Just different people with different opinions on how it should be done.
Michelle
Well, if ccardea's proposal
Well, if ccardea's proposal to designate one person to handle pre-screening is made an "official" part of the workflow then this will not just be an opinion, and that's what I'm referring to. There are a lot of discussions within the group on how to handle reviews right now, and this is one of them. I think its important that we have an understanding and at least some structure to help mitigate conflicts like this.
Considering that this is a collaborative group in which we invite applicants to join in on reviews regularly, it would be difficult if not impossible to designate this type of role and expect any consistency in practice. My point is simply that while there are roles that must be filled in this fashion - like git administrators - I don't think this is one of them.
@ccardea: I think even if there will be a designated pre-screener maybe we should at least designate a couple in order to mitigate any extenuating circumstances in which the pre-screener is AWOL for some amount of time. It could work reasonably if it is documented, though as mentioned above I would avoid discouraging any new reviewers that make this mistake. Regardless of the outcome, I'm willing to take on the role and try to keep on top of pre-screening while this is sorted out.
.
Well, in my opinion, it would be foolish to take the lowest hanging fruit in this process and restricting it to one person. Unless the folks that have taken over this process have a fondness for shooting themselves in the foot?
Michelle
I agree. This is just the
I agree. This is just the type of thing new people can be encouraged to do to get involved right away.
Indeed. I think there is
Indeed. I think there is quite a consensus on this.
Err... Did ccardea leave the
Err... Did ccardea leave the group?
Look good!
My only additional suggestion would have been to provide a link to the 'take over an abandoned module' process, which I see that Greg has done.