Posted by mlncn on November 26, 2010 at 4:49pm
I'm writing that new developers can ask for a code review quite apart from the application review process, but i don't have much i can tell them except ask in IRC and (suggested by beejeebus ask in a g.d.o group on a topic related to your module.
Should we try to create a clearinghouse?
A "code review requested" tag for issues– or projects?
Any other places that exist or ideas for what should or should not be created?
Comments
contributed module ideas
I think there are two groups: http://groups.drupal.org/contributed-module-ideas and http://groups.drupal.org/co-maintainers-projects
Both of those are oriented toward new modules and best practices in coding.
knaddison blog | Morris Animal Foundation
i was thinking more Drupal Tough Love
i don't think either of those groups really address the situation of 'i have a module, it works, but i'd like to make it better'
Maybe make a Drupal Tough Love group? http://www.drupaltoughlove.com/ is essentially what i'm thinking, but open to more reviewers and maybe a little less scary ;-)
benjamin, agaric
Code Review Approach and LDAP Module needing Code Review
We are reworking the the ldap module for drupal 7 and have much of the coding and UI roughed out. Its a perfect time for a code review. Resources such as coder, example, etc. go a long way, but there is no substitute for a code review.
Here's the approach I was thinking:
If anyone is interest in a code review of the ldap module, it would be greatly appreciated. The review would be for structure/architecture at this point. The D7 code is in head and ldap_authentication and ldap_authorization are the modules that are flushed out and ready for review. The ldap_api part is still in the formative stages and authentication and authorization use a placeholder api.
www.johnbarclay.com
Really like the idea of "I
Really like the idea of "I want code review!" as a tag. See the awesome docs mgmt view Lisa Rex built at the Docs sprint a few weekends ago? http://drupal.org/documentation/manage see the "needs technical review" page status? Maybe something like that for project releases?
Yeah, once we have
Yeah, once we have 'sandboxes' as an official content type with the git migration (as I understand it), it would be nice if we could have a 'Needs review' taxonomy term that could be applied to sandbox nodes.
Senior Drupal Developer for Lullabot | www.davereid.net | @davereid
Posted an issue to d.o for tagging of projects
Provide a way for project maintainers to tag or flag their module, theme, or installation profile as "Code review requested"
benjamin, agaric
Going with the issue tag peer-review
Wiki of tags here: http://groups.drupal.org/node/125424
benjamin, agaric
benefit, queue?
I like this idea. But what is the benefit (besides general code improvement) of someone reviewing code like this? Is there some sort of tag that we can put on projects to say this has been reviewed or something like that? And even then, a "review" would be extremely subjective.
Also, where do these requests go? As in what queue?
--
zzolo
Hi zzolo! I hadn't thought
Hi zzolo!
I hadn't thought about any sort of quality notation on the project page, and i think we can start this without any external incentive except the knowledge of getting better code. To begin with though we could develop a standard for linking to the review issue on the project page "Code review by awesome_volunteer", as much about giving credit to the reviewer(s) as assuring users about quality.
More and more i'm thinking the default plan is that the review request go in project's own issue queue, and reviewers can find the issues with a search for an issue tag plus open status.
The Peer Review group has been revitalized and repurposed to help discuss and coordinate this, as Code Review seemed still focused on application reviews. Also the "peer review" tag on d.o issues is free and clear for use!
I am more than receptive to your ideas on this, because i think your involvement is really important for this being successful.
benjamin, agaric
Definitely would want some ways to show this
And two things come to mind as pretty easy to pull off -
There are probably some other things we could do that would help create some incentives here - maybe participating in peer review of code could be factored into a certifiedtorock score ;) ?
But these two things could be pretty easy.
And personally, I'd love to see the peer reviews taking place in the issue queue for the module being reviewed.
FunnyMonkey
I'd be against a badge of
I'd be against a badge unless the only thing it "said" or "inferred" was "I requested my module to be peer reviewed!". I wouldn't give any weight or merit to a badge that said "My module was peer reviewed!" because it indicates no quality at all to me, Morbus. I don't know who reviewed the module, I don't know what they missed, I don't know if they spent 5 minutes or 5 hours (the Drupal Tough Love reviews would take me a good five hours or more to write).
Yeah, Drupal Tough Love was awesome!
RE the badge, I definitely agree that there would need to be some standard in place to make a "badge" mean anything. I should have made it more clear that I was thinking along those lines.
RE:
Yeah, those reviews were awesome. The attention to detail in those reviews are good examples of the bar I would like to see people measuring against.
FunnyMonkey
I was definitely looking at
I was definitely looking at it as more of an opportunity for mentoring, collaboration, etc. if the "old" review process is going to become much less stringent... I think some people really like and learn from having their project's code reviewed, but it seems like we're moving away from having that be part of the process since it's been such a bottleneck.
Automating, and Checks and Balances
I think @Morbus brings up a great point. I am still for badges, but we need to have very specific standards what kind of reviews have been done, or to what extent reviews have been done, otherwise, the metric that can be created from this process would be useless.
Automated Coder and Coder Tough Love reviews for contrib would be awesome and really alleviate a chunk of peer review that is currently subjective. Though I think its really important to have this be represented well on a project page and start to bring this data into searching, filtering, sorting modules.
Past that, there needs to be a specific sort of checks and balance system for subject peer review of code. Not sure best system, but I think its something worth discussing.
--
zzolo