Captcha questions

Hi odegard,

It seems that Trick Question provides very similar functionality. Could you describe how your module differs?
We prefer collaboration over competition, therefore we want to prevent having duplicating modules on drupal.org. If the differences between these modules are not too fundamental for patching the existing one, we would love to see you joining forces and concentrate all power on enhancing one module. (If the existing module is abandoned, please think about taking it over).

There are a few text strings in your module that don't use t() - examples include lines 22-23, 32, 67 and 72 on .admin file. I can see you do use them elsewhere - but it should be used wherever possible to help ensure that modules can be translated.

There is also Drupal Sniffer error for lines 16-19 of your .admin file. This isn't a major issue, but for variable assignments you can go past the 80 column limit ("Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 chars.")

Another potentially duplicated project is http://drupal.org/project/riddler

I am not big on forcing collaboration or rejecting based on vague impressions of duplication, but if your module is not different from these in some significant way, approval may prove to be difficult. Also, as a matter of smart practice, you will want to note the similarities and differences in your project description, even if approved, so that users can make an evaluation as to whether your project will be best for their needs.

Hi odegard,

Thank you for clarifying the differences between your module. I agree that your project name is clearer :-). Given that your readme states that you're considering porting this to Drupal 8 and you appear to want to add additional functionality that will make your module more significantly different to Trick Questions I'm happy this isn't a major issue.

I noticed you've fixed the PAReview errors - cheers :-). However, your fixes to use t() could maybe use some tweaking - you shouldn't normally pass variables to t():

http://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/t/7

For example:

$description = t(variable_get('captcha_questions_description', 'Please answer the question'));

May be better written as:

$description = variable_get('captcha_questions_description', t('Please answer the question'));

Otherwise translations may be requested for user specified values. This is a minor issue though. :-)

I think the code looks generally clean, and looking through the project application check list I don't see any issues. Would anyone else like to comment?

One difference from Riddler is that it depends on the Captcha module.

Yes, and I would consider that a drawback of Captcha questions. The module title suggests captcha integration, for one. For another, the Captcha module has provided the basis for a number of contrib projects that expand the variety and approaches of captcha challenges. The captcha admin interface even allows administrators to select what kind of captcha should be used on each form, including webforms. Duplicating that functionality not only seems unnecessary, but would be a bit of a pain or potential source of conflicts for site administrators who are using other methods elsewhere on their site.

I would not consider this an issue for holding up @odegard's approval for project development, but I do consider it a feature that makes this particular module less appealing than it might be otherwise. (I have not reviewed the code, just responding to the described feature set.)

Hi odegard,

Very nice and simple module and it seems to work fine. Here are a few issues I came across using coder:

captcha_questions.module

• Line 61: Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
  '#description' => $description,
• Line 62: Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
  '#title' => $question,

In the form description markup, I found a minor typo:
You could also do simpley 'What is 1+1?'' but some bots may figure that out.

• simpley => simply
• The additional quote at the end of 1+1? can be removed
• A comma after 1+1?

Otherwise, I don't see any major issues. Good work! :)

Great! I have done reviewing and testing this module and I don't find any issues.

Everything is passing on PAReview. Looking through the code everything seems clean - there are reasonable comments and output seems to go through check_plain when relevant. Overall the quality of this module seems good. I don't see why this shouldn't be marked as RBTC - please let me know if I've missed anything.

manual review:

1. differences to other modules are missing on the project page, please add them.
2. captcha_questions_admin_settings(): doc block is wrong, this is not a hook implementations. See http://drupal.org/node/1354#forms
3. Same for captcha_questions_admin_settings_validate() and captcha_questions_form_validate().

But that are not application blockers, so ...

Thanks for your contribution, odegard!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

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.

Thanks to the dedicated reviewer(s) as well.

Just leave issues at "fixed", they will close automatically in two weeks.