Initial cleanup of module code

Gábor Hojtsy - March 8, 2009 - 06:38
Project:OpenID Provider
Version:6.x-1.x-dev
Component:Code
Category:task
Priority:normal
Assigned:Unassigned
Status:closed
Issue tags:drupal.org identity, drupal.org redesign
Description

I am trying to get my feet wet in terms of what kind of cleanups can we have done here to make it more consistent with the rest of Drupal and work better on Drupal.org as well. This is just a short start.

- Code style fixes: missing phpdoc, @file tag, putting hook_menu() first, wrapping of closing parenthesis for arrays
- UI style: do not capitalize every word on the UI (except maybe in OpenID Provider where I understand it is named like that in the standard and that is the name of the project)
- Fix permission name: this is really about administering the provider, people should not had any other impression
- Fixing hook_user() so that the category title will actually show up (you need to set the #type properly for the right theming, see http://drupal.org/node/254574 with the same issue for cvs.module).

I'd like to do more, but reviewing big dumps is harder then just going iterative, and I'd also like to see the reception of the suggested changes. I believe we can go quick and fix up stuff quickly :) That would make for a pretty nice release if the functionality is right (which I've only tested inbetween Drupal sites :)

AttachmentSize
basic-fixup.patch4.07 KB

#1

Gábor Hojtsy - March 8, 2009 - 06:39

Add tag.

#2

Gábor Hojtsy - March 8, 2009 - 06:40
Status:active» needs review

Uh, has patch of course.

#3

alex_b - March 9, 2009 - 17:08
Status:needs review» reviewed & tested by the community

Looking good. Let's commit this before any other patch that might conflict.

#4

walkah - March 9, 2009 - 17:23
Status:reviewed & tested by the community» fixed

committed, thanks gabor! :-)

#5

System Message - March 23, 2009 - 17:30
Status:fixed» closed

Automatically closed -- issue fixed for 2 weeks with no activity.

 
 

Drupal is a registered trademark of Dries Buytaert.