Username Prefixes

+1, cool useful module!

Only local images are allowed

Hi Fidelix

Nice module!
But here are a few things to work on:
- Could you specify the version of drupal it is made for
- Please put your code in a feature branch and lear out the master branch: follow steps on this page
- Your files have to end with a blank line
- Please use the t() function around all your strings. E.g. 'title' => t('Username Prefixes') I see you used it later on, but you forgot in you hook_menu.
- You still have some coding standard issue:

FILE: ...al-7-pareview/sites/all/modules/pareview_temp/test_candidate/README.txt
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AND 1 WARNING(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
17 | WARNING | Line exceeds 80 characters; contains 90 characters
22 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

FILE: ...iew/sites/all/modules/pareview_temp/test_candidate/username_prefix.info
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
3 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

FILE: ...w/sites/all/modules/pareview_temp/test_candidate/username_prefix.module
--------------------------------------------------------------------------------
FOUND 26 ERROR(S) AND 3 WARNING(S) AFFECTING 26 LINE(S)
--------------------------------------------------------------------------------
1 | ERROR | End of line character is invalid; expected "\n" but found
| | "\r\n"
2 | ERROR | Missing file doc comment
7 | ERROR | Whitespace found at end of line
17 | ERROR | Whitespace found at end of line
19 | ERROR | Whitespace found at end of line
26 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
28 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
28 | ERROR | There must be no blank line following an inline comment
29 | ERROR | Whitespace found at end of line
31 | ERROR | Concatenating translatable strings is not allowed, use
| | placeholders instead and only one string literal
31 | ERROR | The $text argument to l() should be enclosed within t() so that
| | it is translatable
34 | ERROR | Expected "foreach (...) {\n"; found "foreach (...) {\n "
35 | ERROR | Array indentation error, expected 4 spaces but found 6
36 | ERROR | Array indentation error, expected 4 spaces but found 6
37 | ERROR | Array indentation error, expected 4 spaces but found 6
38 | ERROR | Array indentation error, expected 4 spaces but found 6
39 | ERROR | Array indentation error, expected 4 spaces but found 6
40 | ERROR | Array indentation error, expected 4 spaces but found 6
41 | ERROR | Array indentation error, expected 4 spaces but found 6
42 | ERROR | Array closing indentation error, expected 2 spaces but found 4
42 | ERROR | Closing brace must be on a line by itself
48 | WARNING | Format should be "* Implements hook_foo()." or "Implements
| | hook_foo_BAR_ID_bar() for xyz_bar()."
56 | ERROR | else must start on a new line
60 | ERROR | else must start on a new line
65 | ERROR | else must start on a new line
71 | ERROR | Whitespace found at end of line
74 | WARNING | Line exceeds 80 characters; contains 88 characters
75 | WARNING | Line exceeds 80 characters; contains 84 characters
87 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

You can use this url to check your project again: http://ventral.org/pareview/httpgitdrupalorgsandboxfidelix1431768git

Hi Fidelix

- when applying for a full project, you need to mention a few things in your application to make reviewing more easy. You didn't mention the version of drupal it is made for in your issue above. Check this page: http://drupal.org/node/1011698
- The problems with the single new line character are the following. In windows a new line character is two characters (LFCR = line feed carriage return, this is the \r\n mentioned). In UNIX it is a single character (LF = Line feed, this is \n). I guess you programmed on a windows machine. Try to substitute those characters, or try to find the right settings for your editor or IDE.
- The t() function should be used in the l() function, not for the paths, but for the text of your link. If you create a link the path will end up in the href="" attribute and a text will be shown in between . That needs to be translatable.
As you see here: http://api.drupal.org/api/drupal/includes%21common.inc/function/l/7 The l() function takes a text part, a path and an optional options array.
- Line 13 I see this code:

/*
 * Implements hook_menu().
 */

This should become this:

/**
 * Implements hook_menu().
 */

- Please also read the page about Tips for a great project page. And try to add some more information on your project page.

Thanks for you clarification on why you didn't use the t() function in hook_menu. I didn't know that. I guess we can all learn :)

Fix those errors and then you can change the status back to "needs review"

Well, I think it is not the right way. You give a link to go to that page. They will see the breadcrumb on that page. Just give your link a name. Not a whole path to follow, as they don't need to do it.
My suggestion:

'#markup' => t('Roles are listed and chosen based on the order of importance, defined on !link.', array('!link' => l(t('the config page'), 'admin/people/permissions/roles'))),

Not very intelligent image blocking...

http://ventral.org/pareview/httpgitdrupalorgsandboxfidelix1431768git-7x-1x looks good
Code looks good
Tested on D7 and it works like it should

Suggestion:
I would suggest to add a configuration option for the look of the prefix. Like global separator between prefix and username

You should empty out your master branch, so that people don't accidently check out the outdated code inside it ... check out step #5 on http://drupal.org/node/1127732

manual review:

• project page is a bit short, see http://drupal.org/node/997024
• "'access arguments' => array('access administration pages'),": this permission does not fit here. Reuse "administer users" or create your own.
• username_prefixes_username_alter(): roles have a weight, so I think you should not take the rid into account for sorting but the role weight. user_roles() orders them by weight anyway.
• This project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project.

Otherwise I think this is nearly ready.

Your git url is not correct, you should use this one in the issue:

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/Fidelix/1431768.git username_prefixes

How shall "Use Icons as prefixes work"? :)

I'd suggest to adjust the .info file a little bit from "Lets you add customized prefixes to your users" to "Lets you add customized prefixes to your users based on their roles".

Is there any reason why you use admin/config/people/prefix as the config url? I'd expected something lile admin/config/username_prefix.

Did you enhance your module meanwhile to break the 120 lines barrier? Automatical assignment of roles based on a number of comments would be imho pretty useful for community pages.

The problem with the git URL in your initial post is that it points at the "master" branch, rather than the "7.x-1.x" branch. Since your master only has the "README.txt" file, it's not real helpful.

Looks like you haven't added the icon functionality. I could see this being a natural addition to this module, but I certainly don't think it's remotely required for the module to be helpful. This would, however, be an easy way to add a few lines to your code to push you over the 120 barrier.

I agree with your config location of "admin/config/people", though I think it would make sense to name it after your module, I think that's conventional. "admin/config/people/username_prefixes" (or perhaps "username-prefixes") is where I would expect to find it. All in all, not a big deal, just my two cents.

I do think it makes sense to increase your maxlength on the prefixes, it would increase the possible use cases.

Double checked Coder and PAReview and it looks good to me.

I'm marking this RTBC because everything is fine. Granted this is a small module, but it's currently non existent functionality; seems odd to deny access because a useful module is too lean.

Your project page is quite uninformative, please have a look at the tips for a great project page.

Got a config-page? -> set a "configure =" in your .info to the path

Your using variables but your not deleting them when uninstalling the module <- you should fix that before creating a release

But this module looks good enough for me to be promoted.

Thanks for your contribution!

I've promoted this module to a full project and now your able to create releases.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.

The project Username Prefixes has been promoted to a full project.
New URL

Now that this experimental project has been promoted, you'll need to update the URL of your remote repository or reclone it.

git remote set-url origin YOURUSERNAME@git.drupal.org:project/username_prefixes.git