[D7] Debug Tools

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxrogersoper2303945git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Hi Roger,
I think I'm the first human one reviewing the code, so before moving into the tech aspects, if you wouldn't mind, I would like to know the intention of this module.
Of course the "tools" suggests it gives some sort of power-user kind of tools to administer any hidden settings or such, but looking at the code, it's more of "info" module. Either way, I personally see a lot of uses of the module, specially for the clients will less technical knowledge but they still want to send a full report to their IT guy, and such.

Do you have any plans to allow other modules to expose some debug info to your module, and probably reuse the data exposed by other modules with hook_requirements?

I'm going through the module and adding my suggestions. They are not in any particular order though.

- in debug_tools_help, I see you have used filter_xss_admin AND check_plain(), which makes any HTML made through the filter_xss_admin filter gets changed to plain text. You only need either of them (check_plain, filter_xss, filter_xss_admin).

- In function debug_tools_admin(), you are using system_settings_form() function which adds a submit handler to save whatever values the form has generated, minus the security tokens the the Form API adds. Use of a prefix is just a tradition to not mess up with other variables; it will not remove the variables when you uninstall the module.
To remove the variables, usually we implement hook_uninstall in the MODULE.install file and remove the variables using a db_delete with a %LIKE query, and variable_del on each individual variables.

- I see you have email functionality in the module. Generally we use drupal_mail() function to send the email, so Drupal can route the emails obeying the configuration. API doc of drupal_mail() has a lot of information on how to send emails with drupal_mail(). Note that you need to implement a hook_mail() in order for drupal_mail() to work.

- Other than above, there are some minor standards problems that one might not really give much attention. When creating links, always use the l() function. It can take care of the base path, URL aliases, etc. Also, use the t() function whenever necessary. I see you have used it almost everywhere, but a few places are missing the t(). (Good job not using t() in the hook_menu() implementation, which in case you should not use it there).

Also, I would personally love a $SESSION and $_COOKIE viewer.

Good luck!

I'm setting this back to needs work given Ayeshs comments.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

Changes have been applied according to feedback on this page:

https://www.drupal.org/node/2316001

Thank you to those who recommended those changes.

h3>Automated Review

PAReview did see a few problems but quite minor; see the report

Manual Review

This review uses the Project Application Review Template.

I'll have to test this out but I think I'm likely to recommend it to my clients in the future once it is out of sandbox and easy for them to install.

On a slightly irrelevant question, why is this called Debug tools? This is actually providing information and not a tool per se. Do you plan to add any specific tools?

Looks good to me now - marked as RTBC
Works as advertised (though haven't tried browserscape.ini).
All errors identified above fixed.

I did find two very minor issues:

• .module line 58: capitalization: drupal is used instead of Drupal
• readme has a trailing space on a line... extremely minor as well.
• Line 446 still has funicton instead of function but nothing to hold release

Now you have to wait for someone with the appropriate permissions to review the application - step 4 in this list:

1. community members review application
2. resolve issues and back to 1
3. all issues resolved, mark as RTBC
4. review by core team member
5. if they think it looks okay, give you permission to promote it.

Non-blocking issues:

• debug_tools_help() has some duplicate code, checking for slightly different file names, and 2 clauses for the pre tags
• You could move debug_tools_admin() to a separate debug_tools.admin.inc file for slightly better performance (Drupal will then only load that function when actually on your admin page)
• Also in debug_tools_admin(), try not to include any html inside of t(), it makes things much easier for translators
• Also in debug_tools_admin(), use single quotes when possible, they're slightly faster and called for by Drupal code standards
• Consider using Browscap as a dependency
• In debug_tools_create(), calling date() and time() separately should result in very slightly different values. Probably not an issue unless someone files a bug report at midnight
• Also in debug_tools_create() consider putting all the report elements into a keyed array. Then you can build the outgoing message with a foreach(), instead of one line at a time
• Consider making the recipient email an admin configuration. Generally I wouldn't want my users to make that choice when filing a bug report

Thanks for your contribution, roger.soper!

I updated your account so you can 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 stay 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.

do you see a "Promote" tag at https://www.drupal.org/node/2303945/edit ?

Ah, you project is of type "Drupal.org project", but it should be of type "module". You need to delete that sandbox and create a new one of 'Module project' type. https://www.drupal.org/node/add/project-module

function debug_tools_block_info() {
  $blocks['debug_tools'] = array(
    'info' => t('Debug Tools Block'),
  );
  return $blocks;
}

I think you'd better write like this:

function debug_tools_block_info() {
  $blocks['debug_tools'] = array(
    'info' => t('Debug Tools Block'),
    'cache' => DRUPAL_NO_CACHE,
  );
  return $blocks;
}