[D7] Developer Docs

Hi pravin ajaaz,

according to http://pareview.sh/pareview/httpgitdrupalorgsandboxpravinajaaz2467369git-0
please fix the following:

FILE: /var/www/drupal-7-pareview/pareview_temp/developer_docs.module
---------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------
753 | WARNING | Only string literals should be passed to t() where
| | possible
---------------------------------------------------------------------------

one suggestion for all your callbacks/your own functions is to add documentation @param... @return... see: https://www.drupal.org/coding-standards/docs

Also in lines 53-56 developer_docs.module
instead of using:

  $form = '<h3>List of Installed Modules</h3>
  <div id= "search-box">
  <input id="element-class" type="text" name="fname" placeholder="search modules">
  </div>';

create a function to return a form and call it with drupal_get_form

// Blocks created by a module.
    $render_blocks = array();
    $mod_blocks = developer_docs_info_hooks($mod, 'block_info');
    if (!empty($mod_blocks)) {
      $mod_blocks = developer_docs_info_hooks($mod, 'block_info');
      $block_header = array('Title', 'Block ID', 'Delta', 'Status', 'Region');
      $render_blocks = theme('table', array('header' => $block_header, 'rows' => $mod_blocks));
      $pres_elements['block'] = $drup_elements['block'];
    }

    // Node types created by a module.
    $render_nodes = array();
    $mod_nodetypes = developer_docs_info_hooks($mod, 'node_info');
    if (!empty($mod_nodetypes)) {
      $node_header = array('Title', 'Machine name', 'Description');
      $render_nodes = theme('table', array('header' => $node_header, 'rows' => $mod_nodetypes));
      $pres_elements['node'] = $drup_elements['node'];
    }

    // Entity types created by a module.
    $render_entity = array();
    $mod_entity = developer_docs_info_hooks($mod, 'entity_info');
    if (!empty($mod_entity)) {
      $entity_header = array('Title', 'Base Table', 'Bundles');
      $render_entity = theme('table', array('header' => $entity_header, 'rows' => $mod_entity));
      $pres_elements['entity'] = $drup_elements['entity'];
    }
    // Fields created by a module.
    $render_field = array();
    $mod_field = developer_docs_info_hooks($mod, 'field_info');
    if (!empty($mod_field)) {
      $field_header = array('Title', 'Description', 'Widget', 'Formatter');
      $render_field = theme('table', array('header' => $field_header, 'rows' => $mod_field));
      $pres_elements['field'] = $drup_elements['field'];
    }

    // Fields Formatters created by a module.
    $render_field_format = array();
    $field_format = developer_docs_info_hooks($mod, 'field_formatter_info');
    if (!empty($field_format)) {
      $format_header = array('Title', 'Field Types');
      $render_field_format = theme('table', array('header' => $format_header, 'rows' => $field_format));
      $pres_elements['field_format'] = $drup_elements['field_format'];
    }

    // Fields Widgets created by a module.
    $render_field_widget = array();
    $field_widget = developer_docs_info_hooks($mod, 'field_widget_info');
    if (!empty($field_widget)) {
      $widget_header = array('Title', 'Field Types');
      $render_field_widget = theme('table', array('header' => $widget_header, 'rows' => $field_widget));
      $pres_elements['field_widget'] = $drup_elements['field_widget'];
    }

    // Themes created by a module using hook_theme().
    $render_theme = array();
    $theme_info = developer_docs_info_hooks($mod, 'theme');
    if (!empty($theme_info)) {
      $theme_header = array('Machine name', 'File', 'Template', 'Variables');
      $render_theme = theme('table', array('header' => $theme_header, 'rows' => $theme_info));
      $pres_elements['theme'] = $drup_elements['theme'];
    }

Please refactor the above

Suggestion:
put what doesn't repeat in an array, e.g: hook_name, pres_elements index associative, widget_header(title & desc)
e.g:

$elements = array(
  'field_widget_info' => array(
    'field_widget' => array('Title', 'Field Types'),
  ),
  'field_formatter_info' =>array(
    'field_format' => array('Title', 'Field Types'),
  ),
  ...
');
foreach ($element as $key => $element) {
  ....
  ....
}

Let me know if you have any questions, I't be a pleasure to help

BTW, nice module!

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.

Automated Review

All is fine.

Manual Review

Two typos in developer-docs-modules.tpl.php
Line 22: Maintanance should be Maintenance
Line 44: I am not 100% sure but I think "Informations about..." should be "Information about...", I am unaware of any case for using the s in English.

Your code seems to be working as described. However my concern is the size of you functions, I notice two over 100 lines long. Your developer_docs_module_overview function seems like it could be refactored, however at a minimum I think you need to split up the develop_docs.module file.

I am unsure if file size and function size are part of any standard so you may want to wait for a second opinion before you change them.

Looks like a really nice module.
I will take a deep look tonight.

I read the code and found a few strong literals that are not wrapped with t() function. Not a blocker though.

I will write an in-depth review tonight.

You need to associate your git commits with your drupal.org account.
See https://www.drupal.org/node/1051722

Hi Ajaaz!
I think this is a perfect code module. I use phpcs to verify your code and get this:

FILE: C:\xampp\htdocs\developer_docs\css\developer_docs.css
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 1 | ERROR | [x] End of line character is invalid; expected "\n" but
   |       |     found "\r\n"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: C:\xampp\htdocs\developer_docs\css\module_list.css
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 1 | ERROR | [x] End of line character is invalid; expected "\n" but
   |       |     found "\r\n"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: C:\xampp\htdocs\developer_docs\developer_docs.module
----------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------
   1 | ERROR | [x] End of line character is invalid; expected "\n"
     |       |     but found "\r\n"
 181 | ERROR | [ ] If the line declaring an array spans longer than
     |       |     80 characters, each element should be broken into
     |       |     its own line
 190 | ERROR | [ ] If the line declaring an array spans longer than
     |       |     80 characters, each element should be broken into
     |       |     its own line
 199 | ERROR | [ ] If the line declaring an array spans longer than
     |       |     80 characters, each element should be broken into
     |       |     its own line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: C:\xampp\htdocs\developer_docs\js\developer_docs.js
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 1 | ERROR | [x] End of line character is invalid; expected "\n" but
   |       |     found "\r\n"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...p\htdocs\developer_docs\templates\developer-docs-modules.tpl.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 1 | ERROR | [x] End of line character is invalid; expected "\n" but
   |       |     found "\r\n"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

It's nothing important, but please change your end of line character from \r\n(windows) to \n (linux) if you continue developing your code.

Manual Review:

In developer_docs.module
Line 597: $read_me = '';//should remove
Line 135: $render_contrib_hooks => $render_contribute_hooks //avoid error notify.
Line 243: $modulelist => $module_list //avoid error notify.
Line 611: $maint_status =>$main_status // ok?
Line 803: $doclink => $doc_link;//clear name

Hi Pravin,
Thank you for your wishes about the git admin role. It wouldn't be possible without the other senior admins and the welcome was quite warm.
Sorry about my delay in the review. But I could install and use it with many modules.

These are merely suggestions, and are in the order I found them. Nonetheless this is a really good module idea and worked out of the box quite well. None of the points below are release blockers, so I'm setting the status to RTBC.

- Module create date shows 1970/1/1, which indicates the UNIX timestamp must have been zero. Can we hide the date instead of showing the 1970 date. Also, consider using format_date to format the date.

- developer_docs_modules_list() - Output of this function could use renderable arrays. You can use #theme => table to theme tables, and (I know a previous reviewer mentioned the opposite before) the form could use textfield as #theme => textfield. Advantage of drupal_get_form would be the ability to alter it (since it has no submit handlers or any validation), which a hook_page_alter will be able to do if we provide a complete renderable array. drupal_add_* calls can be suppressed in favor of #attached property. There are many modules that do not use renderable arrays, but I thought to mention this now that Drupal 8 uses more and more renderable arrays and it's logically the best way to build the content.

- developer_docs_module_overview(): Output when the module is not enabled, the t() function contains a string literal with h4 tag. It would be better if no HTML is in t() function's string literal. But inline HTML elements (anchor tags for example) are rather encouraged because it gives translators the context.
Also, instead of using $base_path to create URLs, you can use url() to build the URL.

- A few typos (Admininstration -> Administration, etc).

- developer_docs_get_module_info() could use a few checks to make sure the file_get_contents() call was successful, filter_xss_admin() to make sure the README.txt file contents are safe and contains only proper HTML, etc.

- There are some hooks that do not return any data when called. Take block_block_info() for example. If there are no custom blocks, it will return an empty array. However, the way developer_docs_info_hooks() function collects hook information expects data to be there. Initializing and empty array right above the foreach loop should fix the problem. (Go to admin/reports/developer-docs/block page when all errors are enabled. You will see a PHP Notice for undefined variable blocks.

- The module list table could use a unique HTML ID. Then, the developer_docs.js file could use that ID to filter rows. A page can contain many tables with sticky-table class, which can cause troubles with the JS added by this file. Also, it's a general practice to use Drupal behaviors to add event handlers. Make sure you use the .once() notation ($(".elements").once('keysearch').keyup(...).

All above are just small issues, and not exactly blockers. Once again thank you for this, and lets see what others have to say.

Hi Pravin,
Thanks for the additional reviews. They are certainly helpful to maintain the queue, and shows you know the Drupal APIs and best practices quite well.
I think we should leave this application for a few days to let anyone who might have any other reviews to go ahead post them. I will proceed with the approval after a few days if there are no objections.

Thanks for your contribution, Pravin Ajaaz!

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.