[wp-hackers] Code reviews for plugins?

[Eric Mann]

I agree on all points, Mike, but kudos on #3.  Having some kind of objective
checklist is the easiest way to avoid differences in coding or organization
styles.  Things like "organizes all JS code in sub-folders" is far less
vital than "provides backwards compatibility for PHP4" or "automatic cleanup
of added option values on uninstall."

An objective list of criteria also makes it easier for new developers to
start working on a plug-in.  My first attempt broke a lot of "rules"
regarding database setup and access ... I'm ashamed to look back at how I
allowed direct querying of the database via POST variables ... that's a huge
security no-no.  I also never thought to clean up my options fields or extra
database tables until someone actually complained about the extra "crap" I'd
left all over their WordPress installation.  So a checklist that a)
developers can build to and b) an objective 3rd party can verify would be
hugely useful.

~Eric

-----Original Message-----
From: wp-hackers-bounces at lists.automattic.com
[mailto:wp-hackers-bounces at lists.automattic.com] On Behalf Of Mike Schinkel
Sent: Sunday, August 22, 2010 8:21 PM
To: wp-hackers at lists.automattic.com
Subject: Re: [wp-hackers] Code reviews for plugins?

A viable solution seems reasonably clear to me

1.) Crowd-source the code review with a mechanism similar to
http://wordpress.stackexchange.com
2.) Focus on the top 15% of plugins by download and use
3.) Review based on objective criteria (i.e. adds tables and/or uses
existing tables, use of nonces, list of symbols added to global namespace,
number of wp_options added, etc.)

It need not be perfect, it just needs to give some objective and reasonable
facts to allow people to decide their own guidance.

-Mike

On Aug 22, 2010, at 11:08 PM, Mark E wrote:

> 
> 
> On 08/22/2010 03:15 PM, Jeff Chandler wrote:
>> I wrote a post back in 2009 asking if something like what is bring
>> described here is nothing but a pipe dream.
>> 
>> http://www.wptavern.com/is-a-plugin-validation-team-a-pipe-dream
>> 
>> May be worth reading as well as the comments attached to the post. It's
>> a topic and suggestion that keeps coming up but no one seems to know how
>> to tackle without the intricate process of vetting one plugin against
>> another.
> 
> > What happens if a plugin has been reviewed and flagged as
>> awesome but a week later, has a security vulnerability discovered.
>> Doesn't that make the whole system broke and worthless?
> 
> The answer is simple: Ditch the review process, because obviously nobody
involved is qualified to manage it.
> 
> 
> That said, there are issues about code review that people will not agree
on - nor should they. Examples:
> 
> Some people are entirely anal about insisting on the use of camel case.
> 
> Other people are hell bent on using a class even if it only wraps one or
two incredibly basic functions.
> 
> Other people insist that all your css goes in a subdir, all your forms go
in a subdir, all your JS goes in a dir, etc - even if your entire plugin is
entirely user-facing and can be contained in one really really really small
file.
> 
> What all that boils down to is people saying "you need to write code like
ME - I cannot tolerate diversity"
> 
> That sort of thinking has absolutely no bearing in a code review process
unless code might be intended as a core patch, or core addition - that's
different since that code ought to integrate seemlessly into existing style.
> 
> Personally, I don't care how people write code as long as three things
happen:
> 
> - No potential function or class name collisions (e.g. be original and
think ahead)
> 
> - No unnecessary overhead. If code isn't needed yet, don't load it. If you
intend to use data, cache it internally.
> 
> - Think like an intruder and defend your code against becoming an vehicle
for intruders.
> 
> So, in terms of code view for plugins, that's all I'd be looking for. All
the rest is rather pointless.
> 
> That latter security item is the biggest hurdle.
> 
> One really really really big problem with policing anything is that as
soon as you start you accept liability. If you fail at any time, you are
perceived as being liable. On the other hand, if you don't police, no one
can accuse you of doing a bad job and drag you into court over it.
> 
> That's the situation ISPs have faced for about 15 years. And so they don't
police your activity or your content. They might spy on you, and they might
reprimand you if someone brings your activity to their attention, but they
don't police you. And thus they are held liable for what you do. They're
only a conduit - just like the WP plugin and theme repo.
> 
> My opinion, if it isn't clear already, is to forget about screening the
repo. That's not the responsibility of Automattic.
> 
> Mark
> _______________________________________________
> wp-hackers mailing list
> wp-hackers at lists.automattic.com
> http://lists.automattic.com/mailman/listinfo/wp-hackers

_______________________________________________
wp-hackers mailing list
wp-hackers at lists.automattic.com
http://lists.automattic.com/mailman/listinfo/wp-hackers