DarrellDuane [DarrellDuane] [#1013890]

CVS edit link for DarrellDuane

I am applying for Drupal CVS access because I'd like to manage the Bitcoin Address (btc_address) module
as well as other related modules. I've also got my eye on becoming a co-maintainer of a few modules
such as the username_check. I've been developing with drupal for the past three years and using PHP/MySQL
for the past 9 years. I've submitted a variety of patches to Drupal 6 modules over the past two years.
I am currently a freelance Drupal consultant supporting various clients in the Washington, DC
area. I attended DrupalCon DC and SF, and am looking forward to DrupalCon Chicago.

I am a big believer in the power of the open source community to make great things happen and would be
honored to be a part of Drupal's community.

I am submitting the Bitcoin Address as my module. It is also available at http://github.com/DarrellDuane/btc_address
The Bitcoin Address module provides a CCK Field type for holding Bitcoin Addresses. It checks to see if the
Bitcoin library is installed, and if not, it provides instructions for doing this on the Status Report page. It validates
bitcoin addresses that are submitted utilizing this library. There aren't other existing Bitcoin Address modules to
compare this module to. I am aware that there is a module out there that allows for non-programatically creating
customized field types for CCK, but I doubt that it supports validation checking by making a call to a library. It
is much cleaner just to download a module and enable it to make a new field type.

Comment	File	Size	Author
#10	btc_address.tar_.gz	5.82 KB	DarrellDuane
#7	btc_address.tar_.gz	5.84 KB	DarrellDuane
#1	btc_address.tar_.gz	11.19 KB	DarrellDuane

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

DarrellDuane CreditAttribution: DarrellDuane commented 3 January 2011 at 23:29

File	Size
btc_address.tar_.gz	11.19 KB

Here is my module.

Comment #2

DarrellDuane CreditAttribution: DarrellDuane commented 4 January 2011 at 16:36

Status:

Postponed (maintainer needs more info)

» Needs review

Comment #3

DarrellDuane CreditAttribution: DarrellDuane commented 28 January 2011 at 05:13

Component:

Miscellaneous

» miscellaneous

Any news on this? I've got more Bitcoin modules I'd like to commit shortly.

Thanks,
--Darrell

Comment #4

kyle_mathews CreditAttribution: kyle_mathews commented 1 February 2011 at 19:05

I looked through the code and it looks good and cvs maintainer status worthy. Darrell has helped out on one of modules as well and submitted a number of good patches.

Comment #5

tedstein CreditAttribution: tedstein commented 12 February 2011 at 17:23

I just want to clarify that I don't have the ability to give out CVS access, and I am commenting primarily because I have need for a Bitcoin address field type, but I do have a couple comments:

1) Remove all license information. This will be added by CVS.

2) Remove the to-do list from the README.txt. CVS applications are based on finished products.

The code itself seems fine. Hope that helps some.

Comment #6

sreynen CreditAttribution: sreynen commented 12 February 2011 at 17:31

Status:

Needs review

» Needs work

Changing status, per #5.

Comment #7

DarrellDuane CreditAttribution: DarrellDuane commented 12 February 2011 at 20:50

File	Size
btc_address.tar_.gz	5.84 KB

Great points. I've updated the module and fixed both suggestions. The module is ready to use, and I've renamed the ToDo section to be possible enhancements.

Comment #8

tedstein CreditAttribution: tedstein commented 13 February 2011 at 01:36

Status:

Needs work

» Reviewed & tested by the community

I just installed this on a local site, and it is working great. I think this is good to go, but I don't have the ability to grant CVS access.

Comment #9

sreynen CreditAttribution: sreynen commented 17 February 2011 at 14:49

+1. Looks good to me.

The only issues I see are incredibly minor coding standards, e.g. a trailing space on a few lines, a typo misspelling "Implmentation". @DarrellDuane, I'd recommend running it through Coder showing minor warnings to see these issues.

Comment #10

DarrellDuane CreditAttribution: DarrellDuane commented 17 February 2011 at 20:48

File	Size
btc_address.tar_.gz	5.82 KB

Thanks @sreynen. I did run coder on it and had all the errors cleared up in the .module file, but I see the .install file needed some cleanup. I've removed the spaces, etc and now both files have a clean bill of health from coder, and I got the spelling error fixed, great catch.

Comment #11

DarrellDuane CreditAttribution: DarrellDuane commented 26 February 2011 at 05:20

I have submitted this module in my request for permission to commit full projects on Drupal Git.
I've renamed it to be bitcoin_address, I think this is a better name for it.

Please see

http://drupal.org/node/1072411

Comment #12

apaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

CreditAttribution: apaderno commented 28 February 2011 at 06:12

Status:

Reviewed & tested by the community

» Postponed

Please read all the following and the links provided as this is very important information about your CVS Application.

Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for your application. Please read the following documentation on how this affects and benefits you and the application process:Migrating from CVS Applications to (Git) Full Project Applications.

If your application has been "needs work" (or "postponed (maintainer needs more info)"), your application will be marked as "closed (won't fix)". You can still reopen it, by reading the instructions above.
if the status of this application is a different one, it will be changed to "postponed"; you will be able to reopen it by following the instructions in the above link.

Comment #13

DarrellDuane CreditAttribution: DarrellDuane commented 28 February 2011 at 16:48

Project:	Drupal.org CVS applications	» Drupal.org security advisory coverage applications
Component:	miscellaneous	» new project application
Status:	Postponed	» Needs review

My project is till in a good, stable place. I am converting my CVS application to a Full Project application per the instructions provided by kiamlaluno. I'm not sure that I should change the title of this to be th project name as I already have
project application at http://drupal.org/node/1072411.

Comment #14

DarrellDuane CreditAttribution: DarrellDuane commented 28 February 2011 at 17:31

My sandbox project is at http://drupal.org/sandbox/DarrellDuane/1072393

Comment #15

Berdir

German

Switzerland

CreditAttribution: Berdir commented 2 March 2011 at 00:06

- version = "6.x-1.1"
You can remove this line from your .info file, it will be added automatically be the packaging script.

- ; $Id$
Also, this is not necessary anymore, as you can see, git does not use it and it has been removed automatically from all existing projects.

- Only skimmed over the code, but it looks well documented and I didn't spot anything obvious.

Comment #16

DarrellDuane CreditAttribution: DarrellDuane commented 2 March 2011 at 14:54

Thanks Berdir for looking at my code, I've fixed these issues and committed them to my sandbox.

Comment #17

sreynen CreditAttribution: sreynen commented 2 March 2011 at 15:28

Status:

Needs review

» Reviewed & tested by the community

Looks good to me.

Comment #18

zzolo CreditAttribution: zzolo commented 11 March 2011 at 21:08

Status:

Reviewed & tested by the community

» Fixed

Duplicate issue at http://drupal.org/node/1072411

This review was done, in part, at the DrupalCon Chicago Code Review.

You have been approved. Thank you for your patience and dedication on this process. We look forward to your continuing contributions to Drupal and its community. Please read the following pages (and subpages) about Git usage and Drupal best practices:

The following points are things to consider going forward:

$Id$ is not needed.
You still have version in your .info file.
```
bitcoin_address_bitcoin_php_load_library();
```
Should not be in the global module space as it is included on every page. This should be included when needed, such as with the file option in hook_menu(). If really necessary, it can live in hook_init() if really necessary.
Please add more information on the project page for this module as well.

Thank you to the following people for helping with this review:

@kiamlaluno
@kyle_mathews
@tedstein
@sreynen
@Berdir

--
Please be patient with the Full Project Application Review process as it is done by volunteers, we understand that this process is not the most efficient. The goal of the process is to ensure that contributions to the Drupal community remain valuable, and to help applicants build their skills as contributors.