Posted by zzolo on November 1, 2010 at 9:58pm
So, the whole purpose of this group is to create discussion, document, rally around getting more people involved in reviewing new contributors applications.
In Copenhagen, @webchick gave a talk about how this process is pretty negative to most people applying. And, with @webchick's help, we were able to get a BoF going and some short-term, new interest around reviewing application. I even spent a fair amount of time documenting how to review code.
So, the next questions: How do we keep this momentum, and make this a regular thing?
Comments
"negative" ?
Start by asking the applicants, who did not find the process negative and were thankful for the in-depth reviews of their code, about their ideas for improving the process and getting more people involved.
Daniel F. Kudwien
netzstrategen
Manifesto
If you can find me any, I'd love to talk to them. I've only seen the inverse, and lots of it: people who feel discouraged by the process, infuriated by the process, passed over by the process, unfairly treated by the process, and/or out-and-out abandoned by the process. This comes up on IRC on pretty much a weekly basis. (Granted, I'm seen as a "safe" person in the community to raise issues to, so I might proportionately get more of these complaints than most.) Heck, there are people who used to do CVS application reviews who have since stopped because they find the current process so abhorrent.
Most of these "victims" of the CVS application process are now off on GitHub or their own personal blogs or wherever these days and only begrudgingly interact with Drupal.org when their job absolutely requires it. In discussions about how great and inclusive the Drupal community is, they tend to roll their eyes and speak very critically about it. It makes me really sad that such people exist, but not only do they exist, they're growing in number. I see them at every single Drupal event I attend, and some of them even work for first-class Drupal shops.
It didn't used to be this way; it used to be as long as you wrote a halfway decent motivation message and your code didn't contain obvious problems, that was it. You were in. But sometime in the past couple of years, well-meaning folks have tried to introduce the concept of mentorship (which is a great goal on its own) at the time of granting access.
This particular combination is paradoxical and illogical to me. Every other open source project on earth recognizes that the people who actually do something, anything to give back to the project a tiny fraction what they've gained from it are the most infinitesimally small and rare proportion of overall users (the best-case scenario I was able to calculate for the Drupal project was 0.05%) and need to be treated like the precious and priceless resources that they are. They therefore actively go out of their way to reduce barriers to contribution as much as humanly possible.
We do the same too, in most other parts of Drupal. Anyone can provide documentation by editing or adding a handbook page. Anyone can do issue queue triage by updating issues and changing their statuses. Anyone can provide translations through a nice web interface. Anyone can host a local user group and get free promotion for it on groups.drupal.org. Anyone can review patches, and we provide testbot to ensure that stupidly obvious things aren't a problem. Do people sometimes screw this up and do the wrong thing? (adding typos to the handbook or marking the wrong issue duplicate?) Of course they do. And we mentor them on the right way to do things after granting them access, and they go on to do the same for others.
But for some reason, in order to give us new code, we force you to scale a spiky, slippery cliff-side obstacle course that should be reserved only for core patches, since that code runs on drupal.org and whitehouse.gov and needs that level of scrutiny (and as a result, people in the core queue are generally very appreciative of this level of review). The problem is, unlike core developers, these people are generally new to the Drupal community, and this is their first (bitter) taste. The ironic and awful thing is the people they develop a bitter taste in their mouth toward people who are actively trying to help, but it's hard to have that perspective when you show someone this great code you're really proud of that you wrote over the course of the last 3 months and are really looking forward to giving away to others to make Drupal awesome, yet you get told "Nope, that's not good enough" in return. Not once, but several times.
Let's review:
Barriers to Documentation
- Drupal.org account
- Reasonable comfort level with English
Barriers to Translation
- Drupal.org account
- Reasonable comfort level with $other_language
Barriers to starting a local user group
- Drupal.org account
- Search g.d.o to make sure group doesn't exist yet
Barriers to issue queue triage
- Drupal.org account
- Basic reading/writing skills
Barriers to core/contributed patch review
- Drupal.org account
-
patchBarriers to core development
- Drupal.org account
-
diffBarriers to contributing to existing modules/themes
- Drupal.org account
-
diffBarriers to contributing NEW modules/themes
- Drupal.org account
- not just a semi-working, but "release ready" module/theme
- scouring of all drupal.org projects to make sure module/theme doesn't exist yet (and hope it didn't spring into existence while you were making said release-ready module/theme)
- an in-depth understanding of coding standards
- an in-depth understanding of Drupal APIs
- an in-depth understanding of how to write secure code
- application form submission
-
tarorzip- an abundance patience to wait for an incredibly busy volunteer to respond (which, btw, might never actually happen)
- incredibly thick skin to take the comments of whoever reviews your application non-personally.
- more abundance patience to wait for a volunteer to respond again so you can repeat the process again a few times.
- irc (to raise the fact that no one responded to your application once you'd finally resolved all the issues)
and finally, if you're very, very lucky...
-
cvsOne of these things is not like the other. And, alarmingly, the one that's different is our gateway to getting more developers into the project, which provides the very lifeblood of Drupal's future. This is why I refuse to shut up about this issue. What we currently do to new contributed developers is damaging, it's dangerous, and it's actively hurting the project.
Another barrier to documentation
I'd like to add to the list of barriers to documentation: many pages are not editable even by people with a drupal.org account. I have seen many things I would like to have changed (Wikipedia-style) but have not had the rights, even to change things at a page level. Wikipedia, for example, has higher levels of rights for certain things (e.g. large amounts of delete/move), but generally everybody signed-in is able to edit every page, except highly sensitive pages (e.g. things on the home page).
I think the documentation barrier is set too high for people with a d.o. login.
that.points++; Yeah, I used
that.points++;
Yeah, I used to change pages of documentation all the time, but now, I simply can't. I wanted to add a small note about themes in a D7 doc about a week ago, but it didn't have an edit option.
Visit my under-construction site. Let me know what you think. Thanks.
discouraged by the process, infuriated by the process, passed ov
"discouraged by the process, infuriated by the process, passed over by the process".
Yes, I can go along with that:
http://drupal.org/node/976546#comment-3753822
This group is about code review
So I'm not sure that anyone responsible for the documentation team process is ever going to see this.
You'd be better off to ping someone in #drupal-docs or http://groups.drupal.org/documentation-team
I have a mostly-finished
I have a mostly-finished suite of modules that would be useful to a lot of people, and even more useful once everyone gets their paws on it and fixes/adds stuff. It's even a fork of an existing contrib module. My understanding of how to apply for contrib status so I can put it on drupal.org could be charitably described as vague... Plus CVS makes me itch. So I'm one of those over on github: https://github.com/paul-m/metaverse
I'd been waiting for the migration over to git before I really researched contrib status any further... But sign me up as someone who wishes this process were smoother and/or more obvious.
A couple suggestions
Hi,
A suggested addition to your bullet point list "Barriers to contributing NEW modules/themes":
I haven't seen any guidelines or preferences (i.e. which Drupal version is preferred for reviewers).
I'm in the queue myself. It is easy to see the reviewers have a truly daunting task. As discussed here, the number of applications in the queue is overwhelming. The amount of time required to perform a review can't be overstated. Thus as an applicant, I have no complaints :) I simply try to help by reviewing as many applications as I can.
Thus, a suggestion for the main topic here: "How do we get more people to review code?". My suggestion is specific to "module" applications.
I would like to see applicants supply a verification plan. A verification of installation and configuration. A verification the module functions as expected.
In some cases a verification plan details how to create test data. In some cases, the CVS application itself provides test data (e.g. a valid audio mp3 file).
Many of the modules applications that I've considered reviewing, have either a large set of dependencies, and /or external dependencies. For example it would help me if I knew ahead of time how many steps it takes to set up an account with web sevice nnn. How do I verify the web service nnn, independent of the module I am reviewing.
To me, adding a module extends Drupal's functionality. Therefore, when I test and verify a module, that process is analogous to a "Smoke Test" and/or a "reality check" (in addition to testing against Drupal's implementation standards). Thus, an explicit verification section of the CVS application submission, seems apropos.
Finally, I am not suggesting a verification plan as an application requirement. Where applicable, it would be a "nice to have" :)
Hope that helps.
http://public-action.org/
We build online communities
I don't even install applicants modules
Hi @lorinpda. You make valid points and I appreciate your input. Personally, I don't think I have installed any module that I have reviewed. I make the assumption that if the person is applying, and that the code looks good, then it either works, or is good enough to get started on a project page.
On the flip side, I think it would be really helpful to push the culture shift of integrated testing more and more in Drupal. It's still a new thing to a lot of Drupal developers, but is really important in the process of creating stable code.
We also are trying to automate some level Drupal projects (which will include applicants once Git migration happens), like automatic Coder reviews. And maybe just a basic "does this install" check would be good for projects, outside of running the automated tests.
--
zzolo
Great Point
Hi zzolo,
Thank you so much for the feedback. You got the gist of my suggestion exactly :)
My interpretation of the review process is based on Webchick's summary here:
Most terms, by themselves, can be ambiguous. Removing ambiguities is a large reason why we write system documentation. An interpretation of a term is always relative to the context.
What I am getting at is, even the term "release ready" is ambiguous. What type of release? A production release, a Q/A release, a development release? Sorry that is partially a rhetorical question. If I read the rest of the sentence (e.g. "not just a semi-working"), I interpret that to mean "production-release ready"? I'm new, so I don't know whether my interpretation is right or wrong :)
So from my experience, if I am measuring whether a module is "production-release ready", yes, I would be required to install and verify the functions performed as expected.
I hope that helps the conversation along.
http://public-action.org/
We build online communities
agreed
Hi @lorinpda. I agree, "release ready" is not necessarily a good term here. "Beta" might be good. But, I believe the whole idea of specifying is to make sure people don't just apply with an idea or a few stub functions. I think this could be worded better for sure.
--
zzolo
From an applicant perspective
Hi,
I am currently in the application queue and I personally don't have a negative experience. Some thoughts:
- The CVS application process concerns coders, not the code. This can be confusing, even to reviewers.
- There are no code reviews after cvs access is granted so some modules in contrib would fail code review. This led to reviewers reject code borrowed from another contrib module. This could lead to some negativism. "Why they refuse this code while it's allready in?"
- With the upcoming CVS=>Git migration, would it be an idea to move the code review from the version control account application phase to the project creation/publication phase. This would allow a coder to start developing and improving the project. Publishing a project could only be allowed after a code review. This would clean up a lot of contrib and be more welcoming to new drupal coders. It also prevent coders to start development outside of ddo (github, sf.net etc).
- I do notice that coders in the approval queue tend to campaign (ddo, twitter, etc) for their own review. It would be nice to facilitate that and pull in more reviewers.
In my experience the review process a positive experience and is a natural filter for negativism, impatience, arrogance and off course ugly code.
I agree with SqyD, code
I agree with SqyD, code review is a necessity upon every project creation even after CVS access, since it provides more stability and strength to Drupal and the community behind. Currently there are projects created on d.o with no initial commit, there are projects available on the CVS which contains a whole 3rdparty library, there are projects with "Respecting Drupal's coding standard" in their TODO list and there are codes in the CVS that won't pass a parameter to
l()until they get thecheck_plain()output, etc. But there are not enough reviewers out there, I know.Recently I reviewed a CVS application for a pretty useful module, I highlighted where the author needs to change the code but it's more than two weeks & there's no application update. IMO it depends on author coding personality, from his/her point of view it might be hard/time-consuming to change a working (dirty) module either to follow the coding standards or to improve its performance, for example.
Personally I write code for code, I love it and I want it grow, so the whole process seems positive and natural to me, too. By starting this group we can take the first step toward keeping the momentum and makin' it as regular as writing code.
I asked two applicants and
I asked two applicants and got response from morningtime:
On the actual question...
Getting people to review code is difficult. It's a very time-intensive and thinking-intensive process and a good review requires a great deal of skill and broad knowledge about Drupal's internals.
Some ideas:
- Schedule it. Set up weekly or so "code review meetups" in alternating time zones over IRC. This would become dedicated focus time that CVS application maintainers can set aside in their calendars for drilling through the queue together.
- Open it up. Ask the applicants themselves to join these meetings so they can talk directly "face to face" with a reviewer rather than over the issue queue. Revisions can happen faster, and the type of brusque responses common in issue queues tend to get softened when they're delivered in real-time among conversations about cats.
- Promote it. Make up some flyers or something asking for a call for CVS application reviewers (along with a list of requirements) that local user groups can print out and distribute at local meetups.
- Have a list of specific tasks for people to work on. This has been really great for the documentation team and the views bug squad. Looking at a 6-page issue queue with a bunch of crap in it is scary. But being told, for example, "Go through the stuff in the issue queue older than 8 weeks and either close the issues (if we're waiting on them), or bump them (if they're waiting on us)". Having less technical users handling the "triage" would free up brain-space for the very small handful of people who can do the actual reviews to stay focused.
Helping folk produce good modules via guided documentation
I am coming into this conversation from the standpoint of wanting to Improve the CVS Application documentation, by which I mean force applicants to effectively answer yes/no to questions like "Step 1. Have you run your module against Coder" - the sort of things which crop up again and again.
I am also coming into this conversation from a very low skill level of module development (I have only recently submitted my CVS application and learnt stuff already in the process, so I actually appreciate having someone take a look at my module).
Incidentally, even though I have not yet even had my CVS application accepted I have been doing a fair bit of basic triage of CVS applications (seeing if the module works against Coder, looking to see if the module is themeable, seeing if there is a README.txt etc.), so I would certainly say you don't need very skilled people to be adding their 2 pence worth in the early stages (though of course you need the real experts to give the final nod to a module).
When I found this thread I first thought, "ah, no point doing any further work on the docs then, since the CVS application problems will all disappear with Git". Further reading makes me think, "ah, the CVS application process is basically going to be replaced by the "[First?] module release application process".
Doesn't that mean that things are going to get 10 times worse, since currently we check only at the door [=get CVS access], but from soon we will check every time you want to go to the bar [=every time someone wants to do a release]?!?
Improving the CVS application
Improving the CVS application documentation with help both applicants and reviewers. A checklist type could be a good method, but needs testing with new applicants to see if it works. The check list should aim at helping developers to bring their modules a the minimum requirement level step by step.
It also helps new reviewers to see what is required to pass and what is optional for future improvement. As new reviewer I still struggle with this; When do I say 'RTBC'. Documentation could be added to aid the reviewers in this process.
Module Quality Label?
What about separating the process of allowing people to submit modules from the process of assessing module quality?
So, you allow everyone to submit modules (=> large amount of dross).
But you organise things so that modules can be submitted for quality assessment? For example, each module could be given a rating (from say 20), to whether it conforms to the following 20 items:
1 - runs through Coder with no errors
2 - includes a useful README.txt
3 -
etc.
Modules could then display and be sorted according to their "peer quality rating".
Recommend people do the mentorship route: be a comaintainer
So, i've known for a while that people who know the community well enough avoid the code review process altogether by being "sponsored" to become a maintainer of someone else's module... then the approval process can take 11 minutes.
We could point out somewhere that this is an acceptable route to gaining CVS access— approach someone with an existing module and help that person until she wants you to have commit access to her module. This builds in mentorship and gets more people involved in reviewing code (not for original modules, but it's a good entrance into the community for the CVS applicant).
benjamin, agaric
95% of the items I bring up
95% of the items I bring up in a code review are items that would naturally be caught by coder - no @file docblocks, comment format, not using t() on strings, no CVS $Id: $ tags (obviously to be deprecated with GIT), coding style, etc. etc.
I think it would be a good move to force users to satisfy coder requirements before a human reviews their code, leaving the human to primarily check for things like module duplication, code licensing, best practice, mentoring aspects etc. Perhaps create a 'coder' service on a site that accepts uploaded tarballs, and runs coder reports on the uploaded tarball?
Brian Vuyk
Senior Developer, PINGV Creative
bv@pingv.com | (315) 849-9733 | Skype: brianvuyk
agreed 100%
I agree 100% that we need to kind of force people through things like Coder before they reach human intervention:
http://drupal.org/node/977336
I have requested document admin rights to be able to make the required changes but have been refused:
http://drupal.org/node/976546
In addition to being told I cannot be granted admin rights to make the changes which are a no-brainer to me, and therefore requesting those who do have access to make these changes, I am told that these changes will not be made:
http://drupal.org/node/977336#comment-3756298
I agree with you 100%. However, those who hold the power will not do the work nor let in those who want to make the changes. I am powerless. It's very frustrating.
comming from a current cvs applicant.
I would agree there are some really great ideas here. Running the module through code on upload would greatly speed up the process.
Currently my module is still in the review process. I uploaded my module on December 4th 2010. At first the process went smoothly, I got some response immediately on my application and I was quick to respond to those concerns. The only thing I have seen a code reviewer do on my module is switch the status when needed. I have been lucky enough to have someone from the.community take some time and review my code. They have been very helpful in working out a few bugs. But again its been almost a month and haven't seen any action on my application. I don't know if this has anything to do with the git migration, but I have started to become very discouraged. I love using drupal and being apart of the community, but this has left a sour taste.in my mouth. I don't want to sound negative or ungrateful, just makes me feel unimportant. I have sense then written 4 othere great modules that I think a lot of people would use, but they just remain on my laptop getting use only from me and my clients. Well I just wanted to share my experience with the process and hope I maybe gave some insight. If anyone would have a minute I would graciously appreciate some time on my application http://drupal.org/node/938726. Thanks in advance!