CVS edit link for Jimmyax

I tried to find a module limiting the allowed characters for usernames to the classic IRC characters. I could not find a module so I have written my own.
CommentFileSizeAuthor
#13 irc_nick.tar_.gz1.3 KBJimmyAx
#10 irc_nick.tar_.gz1.29 KBJimmyAx
#8 irc_nick.tar_.gz1.23 KBJimmyAx
#6 irc_nick.tar_.gz1.56 KBJimmyAx
#2 irc_nick.tar_.gz10.17 KBJimmyAx

Comments

JimmyAx’s picture

JimmyAx commented
Status: Postponed (maintainer needs more info) » Needs review

Here is the module I have written.

JimmyAx’s picture

JimmyAx commented
StatusFileSize
new irc_nick.tar_.gz10.17 KB

Here is the module I have written.

Edit: Sorry for the double post but varnish gave me some temporary error when I was uploading the module.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

Hello, and thank you for applying for a CVS account. I am adding the review tags, and some volunteers will review the code, pointing out what it needs to be changed.

As per requirements, the motivation message should be expanded to contain more features of the proposed project. For themes, it should include also a screenshot of the theme, and (when possible) a link to a working demo site; for modules, it should include also a comparison with the existing solutions.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: +Module review
JimmyAx’s picture

JimmyAx commented
Status: Needs work » Needs review

As far as I'm concerned there are no modules matching this one. The closest I know is the in-core access rules, but that one is just limiting the allowed usernames, not limiting the allowed username characters.

JimmyAx’s picture

JimmyAx commented
StatusFileSize
new irc_nick.tar_.gz1.56 KB

Update:

  • Corrected the expression used to match users.
  • Moved the description to own function.
  • Removed the .git trash that was included in the previous archive.
brianV’s picture

brianV commented
Status: Needs review » Needs work

Some notes:

  1. Please remove LICENSE.txt. It is added automatically by CVS.
  2. Please remove version = 0.2 from the .info file.
  3. Please add an @file docblock to the top of the .module file. See http://drupal.org/node/1354#files

Once those are accomplished, this should be good to go.

JimmyAx’s picture

JimmyAx commented
StatusFileSize
new irc_nick.tar_.gz1.23 KB

All modifications done as requested :)

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review
JimmyAx’s picture

JimmyAx commented
Status: Needs review » Needs work
StatusFileSize
new irc_nick.tar_.gz1.29 KB

Update:

  • Fixed warning that appeared.
  • Fixed a user name checking bug.
  • Gave the actual checking a very own function.
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review
brianV’s picture

brianV commented
Status: Needs review » Needs work

One other minor thing I see:

Please remove the copyright statement from README.txt - as soon as you accept a patch, it is no longer correct. Plus, there is a blanket copyright on contrib such that copyright of all code is held by it's respective authors.

Once that is corrected, this application should be ready.

JimmyAx’s picture

JimmyAx commented
Status: Needs work » Needs review
StatusFileSize
new irc_nick.tar_.gz1.3 KB

Once again updated as requested.

brianV’s picture

brianV commented
Status: Needs review » Reviewed & tested by the community

Perfect!

JimmyAx’s picture

JimmyAx commented

There has been almost 24 hours since this once was marked as "reviewed & tested by the community". I can see that there were other issues being marked as fixed within these hours. If there is anything blocking this please let me now, otherwise I just assume this one was forgotten/missed.

zzolo’s picture

zzolo commented
Status: Reviewed & tested by the community » Fixed

@Daniele, thanks for the application and patience; please be aware that this is all done by volunteers. The following are personal suggestions for a better module given my experience in the Drupal community, but do not block you from getting CVS access. Happy hunting!

  1. Describe in README what the "classic IRC characters" are. Also, maybe why someone would want to use this module.
  2. You could really tighten up the code a lot by not using so many functions and variables.

Please read the following resources to make sure you know how to use CVS and the specifics to the Drupal CVS infrastructure, as well as how to be a good module maintainer on Drupal.org. The Drupal community is very large and dynamic; we welcome you as a module maintainer and hope that you embrace and challenge the Drupal community and continue to contribute.

--
Note: Please be patient with the CVS application process. It is all done by volunteers. Our goal is not to be arbitrarily slow or meticulous. Our goal is to get you CVS access and ensure that you are and will become a more responsible Drupal contributor. For a quick reference on what I look for in a code review, please see this article, or read the handbook page on how to review for reference..

zzolo’s picture

zzolo commented

Hi @JimmyAx, sorry for the name mixed, the message above was indeed intended for you.

Status: Fixed » Closed (fixed)
Issue tags: -Module review

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

Issue tags: +Module review

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Component: Miscellaneous » new project application
Issue summary: View changes
Status: Closed (fixed) » Fixed

Status: Fixed » Closed (fixed)

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