@klausi, many thanks for responding to previous threads relating to the issue of “Pareview Bonus - requirement for reviews”.
For others reading this, I should say that Klausi has advised me to raise a new discussion here, rather than mentioning it in overlapping discussion threads.
The gist of which is that many people, including myself, feel that requiring new contributors to do a certain amount of reviews is a good way to get them involved and relieve some of the backlog. However, there is a level of frustration with the process, which I can partly summarize as follows:
- New contributors are not best placed to do reviews (although not all new contributors are novices).
- Less experienced contributors tend to have their issue set back to “Needs Work” more frequently and are therefore required to do more reviews of other projects, than more experienced contributors. This is unfair, and understandably people are reluctant to say anything as they await approval.
- As a result, there is an uncertainty at the outset as to how many reviews a contributor will need to do (above the minimum 3 stated), in order to satisfy the review bonus requirements. It should be clearer.
- The prior advice of sticking with one issue is, to all intents and purposes, defunct (but it shouldn't be). To illustrate: 3 follow-up comments on one issue count as 1 review, whereas 3 cursory comments on 3 separate issues count as three reviews. There is a feeling that a lot of these cursory comments are just going through the motions, and add little of value.
- There is a level of frustration for those that receive cursory comments on their issue, particularly if it is apparent that the comments are just satisfying the Review Bonus requirements. This is easily checked by looking at a person's open applications to see if they are awaiting approval.
Here are some examples of issues which required between nine and fifteen reviews to be done:
https://drupal.org/node/1941468
https://drupal.org/node/2059401
https://drupal.org/node/1884178
Although, it has been pointed out that the number of issues required to do more than 6 reviews in a small minority, this doesn't make it fair and Drupal.org should, above all, be seen to be fair. If the average is in fact 6 or less, then setting a limit should have little or no effect.
People who are prepared to get involved with the review bonus system show more willingness than those who don't. Most of us are aware that employers are seeking some evidence of contributions to Drupal.org and I fell that we must make the community as welcoming as possible, and make their path to publishing their project smoother. It is for that reason that there should be a refinement to the review bonus program, which although it is your baby, has become the de-facto way to progress a project application quickly. It is perhaps time to elevate it to encompass all reviewers.
I found the review bonus program to be a positive experience and full credit for putting it in place. I do however feel that it is such an integral part of the application process that it needs to be improved a little:
- Decide if requiring one contributor to do more reviews than another is fair.
- Decide if sticking with one application is better than multiple applications, i.e. a review should count as a review, no matter whether is on singular or multiple issues.
- Depending on 1 above, set a limit of say 6 reviews (after which the review bonus tag remains) and update advice to reflect that.
- Discuss whether all reviewers should become party to this bonus system.
- Update advice and create a “What to expect form the Bonus Review Process” guidelines.
- Create a one-stop feedback section for the bonus and normal application review process, with scheduled improvements.
Actionable Items
I am, of course, happy to do any work necessary to help out with any changes.
Thanks in advance for considering my suggestions.
Comments
To clarify: follow-up review
To clarify: follow-up review comments of course count as separate reviews. So If you review an application more than once all your review comments count. That's why the documentation says to put the exact link to the comment into your review bonus section.
Did you find documentation that says to stick with one application? That should be removed, because in practice the applicants can take a long leap of absence before getting their projects ready for review again. So reviewers have to turn to other applications in the meantime. So actionable item 2 is already decided, you have no choice but to review multiple applications.
Your actionable items are not really all actionable items. One would be item 3 that has a concrete proposal how to refine something. Deciding whether something is to be considered fair or not is hard to do, and what would be the outcome? Documenting on the review bonus project page "we have decided that this is not fair"? Or that we stop the review bonus program and go back to the state where applicants don't get any feedback for months and we review the oldest application? This process will be "unfair" one way or another, but I believe the review bonus system has made it a lot more fair.
Regarding "all reviewers should become party to this bonus system": reviewers can decide themselves whether they want to participate in the review bonus program or not. Why should we force them? Also, the review bonus list is empty most of the time, so we rather need git admins that take a look at the other RTBC applications.
Regarding "What to expect form the Bonus Review Process guidelines": that can be added to the general "what to expect" page at https://drupal.org/node/539608 or to the review bonus page at https://drupal.org/node/1975228 . Feel free to edit!
"Create a one-stop feedback section": that already exists at https://groups.drupal.org/node/142469 and https://groups.drupal.org/node/142464 . There is a TODO to provide anonymous feedback, that would be interesting to look into.
I raise an exception
I totally disagree. When I had a review and I challenged the review, explaining why I did things in a non drupal way, I rarely received an answer.
A reviewer who does not answer a challenge? That review is useless. It's been reduced to an opinion with the same value as a barrel of hair.
If you want to foster a master apprentice relationship to maintain and improve the quality and integrity of the craft, then you encourage the masters to stick with capable apprentices until they graduate.
If an applicant challenges a review, then the associated review bonus should be set aside until answered. Every Drupal member who has git vetted status should be required to do 1 or 2 reviews (or promotions) a year just to maintain their professional status.
Fairness is pretty clear cut,
Fairness is pretty clear cut, and I don't think its difficult to decide if it's fair to require one person do do more reviews than others in order to gain the same result, it simply isn't. However I'm sure this isn't deliberate, just a side effect of the process.
There has been previous guidance that reviewers should stick with a project until completion, however my comment is based on active issues, I didn't suggest that people stick with one issue even if it is unresponsive, and only one .
The point was that multiple comments on one issue should count towards pareview requirements, currently people think they need to do separate reviews as evidenced by the links posted in those issues.
Currently, you are the owner of the pareview bonus system and what you decide is what goes, which is fine as you re doing the work, but I had suggested rolling it out as an overall bonus system, for optional use by all reviewers of course, but to be an official review bonus process.
You were the one who requested that I post actionable items, which are just suggestions, but I can't help feeling that there is significant irritation at my proposals, as if I am somehow interfering.
Ah, I think i misunderstood
Ah, I think i misunderstood the actionable items and interpreted them as separate points. So you just want to declare a maximum limit of reviews people should be told to do, correct?
I think I can relate to that and now I also better understand your point about fairness. I can see that people can get pissed off if they go through many iterations in their own application.
So setting the maximum limit to 6 reviews sounds fine with me, +1 on that proposal. I'm going to keep to review bonus tag on applications that have at least 6 manual review comments from now on. Would you like to document that on https://drupal.org/node/1975228 ? Anything else you would like to write down (you were mentioning a "what to expect" part)?
Please also note that this change would not affect any current open application, since the review bonus program is running a bit low on applicants right now.
Thanks
Klausi, it seems that my comment "Just to clarify:" below overlapped with yours above, as I was typing and hadn't refreshed the page in about 30 minutes.
Thanks for agreeing to that, I am happy to amend the documentation which I'll do today.
The only other point is that of multiple comments vs separate reviews, I'd welcome your advice on whether you think the limit should require 6 separate reviews, or 6 rounds of review i.e. on one or two issues?
Here are my own thoughts from
Here are my own thoughts from someone new to this process on D.O but with a lot of experience outside of D.O.
The project review process should have more defined requirements. A lot of the steps and things talked about are very "fuzzy" at best.
PAReview.sh: Wonderful automated tool for code reviews. Make it a a requirement! I personally reviewed a module of someone waiting for feedback that didn't even make updates from the PAReview till I told them how to make the changes. Enforce that all PAReview items must be addressed or explained in the project application before an application gets the "time-of-day". That's pretty easy IMHO. They are after-all the coding standards of Drupal - they SHOULD be a requirement for any and all projects. Possibly state that all items market as "Errors" must be fixed.
Now onto the items brought up in this thread by @davidmac under Actionable Items
1.) Why not set a higher minimum and stop removing the PAReview bonus tag completely. Once you have done your duty, you are done. You don't get called back to the front lines anymore. The current process discriminates against newer developers. Also why I say PAReview.sh should be a hard-fast requirement.
2.) I think @klausi answered that one
3.) Mimics what I said in #1. Raise the limit and stop removing the tag. Requiring three reviews because you got a single line of code wrong is going to do nothing but piss people off and turn them away.
4.) I think reviewers should make up their own mind about it. I personally didn't let the PAReview bonus tag sway who I reviewed. I went after things that I had domain knowledge or a lot of experience with - basically something I could review with confidence and give valuable feedback. IMHO that is a better approach for a reviewer, the quality of reviews could be increased if one thinks that way.
IMHO This statement: "Pick the oldest application from the review bonus list and get started! (If it is empty pick the oldest from the "needs review" list)" from this page https://drupal.org/node/1975228 I think should be changed to read:
Choose your strengths first to give good and valuable feedback to others. Next attack the PAReview bonus tagged and old item queue.
Lets not sugar coat this and be brutally honest, the end goal is to produce good code on D.O - fostering developers is a secondary goal and side affect of goal #1. A developer seeking project status should be getting the best help he can to make sure his project is sound and solid.
5 & 6. So there are a lot of places to edit documentation.
These are my opinions. My biggest gripe is with the technical debt that spans all of what is on D.O
All this being said, I will continue to dedicate some time each week - my lunch break on some days - to review projects in the queue.
Brady
@fastglassllc
@derailedonline
Here's an old idea...
That's the best reason of all to use these tools on every existing contributed module -- like some kind of barometer of health. Lot's of people have suggested this. Many think it would be a good thing.
But face it, for more than a few privileged members, this will be like going for a rectal exam. It just ain't gonna happen.
Just to clarify: @klausi, On
Just to clarify:
@klausi, On the one hand you interpreted my suggestion to make the Bonus system official as a suggestion to force other reviewers to use it, which is not the case. Yet, you have previously proposed making it strongly recommended (https://groups.drupal.org/node/218754), which must mean we are thinking along the same lines. The prominence of the review bonus system means that it is fast becoming a standard way to progress applications, yet people still refer to it as being somehow unofficial and not mandatory (https://groups.drupal.org/node/195848#comment-988398) as a barrier to it's improvement. How can it become official?, what is the procedure by which something becomes official?
@perignon, I agree that we need to update the advice on what happens in relation to the number of reviews required versus the removal of the Pareview tag as there is still uncertainty(https://drupal.org/comment/5557410#comment-5557410) which could be improved upon. The wider understanding is that at least 3 separate reviews are required and there is no advice that multiple reviews (sticking with an issue) count as more than one review when it comes to the review bonus requirement. Telling people to provide a link to the exact comment does not explain this one way or the other, it simply says provide an exact link. The advice
… refers to multiple reviews and “them”, so I think clarification is necessary.
I hope it is clear that my desire is to see an improvement in the process is directed at the process only, and if just one individual is required to review 15 projects versus another who is required to review only 6, then this is one too many. For others who look through historic applications prior to submitting their own, this can be very daunting.
Updated Review Bonus Documentation
As promised, I have updated the review bonus documentation here:
https://drupal.org/node/1975228
I have explained that reviews should be done on separate distinct projects, which seems fair given the new limit of 6.
Reflecting on previous comments, I can see that allowing reviewers to use multiple comments from a single issue is open to manipulation (i.e. breaking up feedback into multiple comments) so I've clarified it as a requirement for separate reviews, in line with current practice.
As ever, your feedback is welcome, I hope you think it's an improvement, and I will look into helping out with more documentation going forward, although I've included a "What Happens Next" section to explain the one time removal of the Pareview: review bonus tag.
Thanks, that looks good! IMHO
Thanks, that looks good!
IMHO people don't need to review separate applications - if they provide another good review after the maintainer has fixed stuff from their first review I think that is fine, too.
Not sure how I could express that in a meaningful and non-confusing way on the doc page.
Fine, but to clarify ..
@klausi
Thanks.
When you say another good review (on the same issue) presumably you mean another on each of the three reviews already submitted. Can you clarify?
I had originally suggested that separate reviews were too much and sticking with an issue was already advised. The trouble is that it's hard to monitor. Going back to my point above, what's to stop people splitting one good review into two mediocre, but acceptable comments, and thereby halfing the review requirements.
I am willing to amend the documentation, if we can arrive at a concensus.
In general I think looking
In general I think looking for something "fair" in open source is a bit of a quixotic quest. The question becomes one of being Fair for whom? and then you can almost never actually answer it.
To provide a counterpoint: as a reviewer I get (got) frustrated by people who post a project, get a review with detailed requests, claim to have fixed things (without links to commits, without specific descriptions of which items are fixed or not) and mark the project as "needs review" again without fixing everything. In that case, I don't mind the fact that someone might get moved back to "needs work" and have to do more reviews in order to get the review bonus to get another review. When they are not thorough in their fixes they have wasted the time of the reviewers and doing a few extra reviews to offset that seems fine to me.
I'm also not sure I see why them being "new" is a problem (point 1 of the original post). It's true that new reviewers may give incomplete advice, but giving advice and responding to feedback is part of the process of learning. The focus of this process is not just making someone's first module good but instead it is really about learning. I believe each additional review done in the process (for any reason, including The review bonus system) helps increase that learning.
What about gradually reducing the hurdle? Something like "to get your first review bonus you have to do 3 reviews. The second review bonus takes 2 reviews. The third and subsequent review bonuses requires 1 review." I worry that someone who can keep the review bonus after 6 reviews will 1) be learning less and 2) not have as much motivation to actually fix things.
knaddison blog | Morris Animal Foundation
I don't agree that fairness
I don't agree that fairness is hard to come by, in open source or in general, it just requires some empathy with new contributors.
I think we have to delineate between those who are prepared to embark on the review bonus system and those that are not. If they help out 'at all' they shouldn't be further penalised 'over and above' those in the issue queue that don't get involved with the bonus system, but whose application will eventually get reviewed without any additional requirement placed on them, albeit less quickly.
It's about defining a clear expectation from the review system and I agree that there can be frustration when people ignore advice (not always deliberately) and don't make the required changes, but I think repeatedly setting it back to "needs work" is enough motivation for them, as their issue won't progress until the code is right.
The Review Bonus has been updated to account for 2 phases of 3 reviews up to a maximum of 6. I personally don't see that adding in more complexity will attract users to it, in fact if it's simpler, more people might use it IMO. But perhaps klausi has other ideas on it.
Good question
It is NOT fair that this process is applied only to new applicants, regardless of their experience or commitment to Drupal. Everybody else gets a free pass.
There is a lot of cruft in the DO closet. Now we have a vacuum cleaner but it won't be used. Some people hate the noise.
Tangible Equity
You have a point, and when people start to argue that's fairness is somehow intangible and impossible to quantify, it indicates significant resitance to change, why else would suggestions for improvements to the process receive such flak.
need more how, not why
I disagree. I think it actually indicates the exact opposite of that. Fairness discussions don't often lead to change. The people pushing back know this and they'd prefer to see actual change. For example, they've seen dozens of discussions about how unfair it is to only review new contributors, but no practical plan for expanding reviews to all projects, a monumental scaling problem.
Let's assume for a moment that we all agree on what is fair. What exactly should we do next? Those are the discussions that lead to actual change. You think we should review all projects? Great, how do you propose we make that happen? You think reviews should be allocated differently? Great, I can tell you how I do that.
I suspect if you start with how instead of why, you'll find everyone is more eager to discuss. In a community full of volunteers, we have plenty of why already.
Well the opposite has happened.
Despite what you pupport, this fairness discussion has led to some minor change by virtue of the new limit on the number of reviews required per person, despite the dust it has kicked up.
Great post, Scott. I think
Great post, Scott.
I think you touched on my frustration with this entire discussion ... it's spinning cycles on the wrong problem.
Want the same standards applied on all projects? I'm fine with that, but it isn't going to happen via manual reviews. Modifying the 'review bonus' program isn't going to help either ... the key is how we decouple ourselves from the 'human volunteer' requirement which prevents this entire process from scaling; instead of how we can patch the flat tire.
There is a lot of cruft in
Great aphorism! I been running PAReview over a lot of projects on D.O and it's not pretty.
Brady
@fastglassllc
@derailedonline
Worth noting here. Just had a
Worth noting here.
Just had a post on a project I was reviewing. https://drupal.org/comment/8188151#comment-8188151
Brady
@fastglassllc
@derailedonline
Thank you for that link Perignon.
I just did my first project review. Didn't look at the code. Didn't have to. Of course, that is just my opinion.