What to do when module maintainer refuses to follow coding standards?

I believe it's this issue

So, the question is what should be done when the maintainer is "unresponsive".
Last time it happened, chx assigned the person who filled the complaint as the co-maintainer, which solved the problem.

I do not think that coding standards are something that should be actively enforced if a module maintainer chooses not to implement them. The fact is that right now, I would bet that no less than 90% of contrib contains coding standards violations of greater or lesser extent. I myself have committed modules that willfully ignore the stupid 'space, nospace' standard for string concatenation. I think hefox took the right approach by suggesting that they be fixed, and then offering a patch to do so is even better. Ultimately, however, attempting to enforce the issue is the wrong approach imo, and more than that simply impossible and any attempt to do so will look arbitrary and capricious because the victim will always be able to say 'Why are you picking on me and not ' There's no way down this road that isn't more trouble than its worth.

It depends on how severely it violates them. For minor violations, I don't pay much attention. (Views has always used the "space on either side of the concatenation dot" convention, which wasn't the case for core until D7 for instance.) For major violations, well, I have actively refused to use a module before on those grounds. If the maintainer won't follow something as simple as coding standards I don't trust their ability to follow security standards. It also slows me down as I can't parse the code myself as easily.

I think we should have requirements and optional elements for code hosted on drupal.org

GPL V2 and later License? That's a requirement.
Respond within X amount of time to a security issue reported in your module? That's a requirement (though the exact length is still being decided).
Coding standards? Those should be optional but highly encouraged, in my opinion.

At the risk of introducing an unenforceable grey area, I think they should be encouraged but not forced unless the coding style is completely unreadable. If the code is so bad that the security team can't make heads nor tails of it, that's a problem. But tabs instead of spaces? That doesn't seem such a big deal in contrib.

There's only so much you can ask of volunteers before you end up with an "f that" and I'd rather see an actively maintained project with tabs in it than one with perfect coding standard following that's unsupported. We need to choose our battles and I don't think this is a good one to choose.

Michelle

Code style is one of those things that are a matter of habit, and very hard to change. I struggled with Drupal's code style early on myself, because I was more used to braces being aligned on columns, not K&R C style, and other minutiae. Of course, now, after so long, it is second nature to me.

The point is: some people will be used to other coding styles, and feel too strongly about it (think vim vs. Emacs, KDE vs. Gnome, ...etc.). If they continue to contribute to Drupal, they may change their style after a while.

We should strongly recommend that they use the consistent style that core and the majority of contrib uses, for their own benefit (patches are easier for others, ...etc.)

Coding style alone should not be enforced, just prodded. If it is combined with other problems, sloppy programming, security issues, weak maintainership, then it is yet another sign of a questionable maintainer, whose module should be avoided. Alone, it is just what they are used to.

I am perfectly fine with people hosting their code elsewhere if they do not want to integrate with the community in such easy things as coding standards.

I flatly refuse to review code or patches that grossly deviate from the coding standards.

@bangpound, here's a start: http://drupal.org/node/539608

@Crell, decide for yourself how bad this is:

$expiry_data = db_result
(
        db_query_range
        (
                'SELECT fpce.expiry ' .
                'FROM {force_password_change_expiry} AS fpce ' .
                'LEFT JOIN {users_roles} AS ur ' .
                'ON ur.rid = fpce.rid ' .
                'WHERE ur.uid = %d OR fpce.rid = 2 ' .
                'ORDER BY fpce.weight ',
                $user->uid,
                0,
                1
        )
);

As the one who submitted the patch, I can heartily attest that the current state of the code is almost unreadable.
I occasionally forget to put spaces on either side of concatenations, but this is deliberate.

As I said, there are many code styles around, and having been immersed in one style for years, the new style looks foreign, unfathomable and a real obstacle.

Integrating with the community takes many forms, and adhering strictly to the code style should not be one of them.

You can open an issue, talk to the person via email or IRC, blog on it, refuse to use the code, ...etc. All that is fine.

But don't make this, alone, a reason to ban hosting of the code on d.o.

Mayhaps this should be discussed at DrupalCon CPH: http://cph2010.drupal.org/sessions/rights-and-responsibilities-maintaine...

Did this come up in CPH?

Related: #891044: Consistency in CVS guidelines.

Maybe this should be discussed at DrupalCon Munich.

Guess nobody found it important to discuss this.