[D7] Counter API

You got some coding standart issues (See http://ventral.org/pareview/httpgitdrupalorgsandboxethanw1369794git, you can use this site to re-test your self)

If you got any questions on that please ask!

Review of the 6.x-1.x branch:

Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):

FILE: ...al-7-pareview/sites/all/modules/pareview_temp/test_candidate/README.txt
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
22 | WARNING | Line exceeds 80 characters; contains 87 characters
--------------------------------------------------------------------------------

FILE: ...test_candidate/plugins/export_ui/counter_api_ctools_export_ui.class.php
--------------------------------------------------------------------------------
FOUND 34 ERROR(S) AND 1 WARNING(S) AFFECTING 28 LINE(S)
--------------------------------------------------------------------------------
3 | ERROR | The second line in the file doc comment must be " * @file"
9 | ERROR | Class name must begin with a capital letter
9 | ERROR | Class name must use UpperCamel naming without underscores
13 | ERROR | Method name "counter_api_ctools_export_ui::set_value_page" is
| | not in lowerCamel format, it must not contain underscores
30 | ERROR | Space found before comma in function call
43 | ERROR | Missing comment for param "$form" at position 1
44 | ERROR | Last parameter comment requires a blank newline after it
44 | ERROR | Missing comment for param "$form_state" at position 2
49 | ERROR | Method name "counter_api_ctools_export_ui::set_value_form" is
| | not in lowerCamel format, it must not contain underscores
67 | ERROR | Missing comment for param "$form" at position 1
68 | ERROR | Last parameter comment requires a blank newline after it
68 | ERROR | Missing comment for param "$form_state" at position 2
70 | ERROR | Missing comment for @return statement
72 | ERROR | Method name
| | "counter_api_ctools_export_ui::set_value_form_validate" is not
| | in lowerCamel format, it must not contain underscores
81 | ERROR | Missing comment for param "$form" at position 1
82 | ERROR | Last parameter comment requires a blank newline after it
82 | ERROR | Missing comment for param "$form_state" at position 2
84 | ERROR | Missing comment for @return statement
86 | ERROR | Method name
| | "counter_api_ctools_export_ui::set_value_form_submit" is not
| | in lowerCamel format, it must not contain underscores
96 | ERROR | Method name
| | "counter_api_ctools_export_ui::increment_value_page" is not in
| | lowerCamel format, it must not contain underscores
126 | ERROR | Missing comment for param "$form" at position 1
127 | ERROR | Last parameter comment requires a blank newline after it
127 | ERROR | Missing comment for param "$form_state" at position 2
132 | ERROR | Method name
| | "counter_api_ctools_export_ui::increment_value_form" is not in
| | lowerCamel format, it must not contain underscores
139 | WARNING | A comma should follow the last multiline array item. Found:
| | "The value of the counter as of this page being displayed.
| | Note this might change between the time this page is generated
| | and when you submit this form."
157 | ERROR | Missing comment for param "$form" at position 1
158 | ERROR | Last parameter comment requires a blank newline after it
158 | ERROR | Missing comment for param "$form_state" at position 2
160 | ERROR | Missing comment for @return statement
162 | ERROR | Method name
| | "counter_api_ctools_export_ui::increment_value_form_validate"
| | is not in lowerCamel format, it must not contain underscores
168 | ERROR | Missing comment for param "$form" at position 1
169 | ERROR | Last parameter comment requires a blank newline after it
169 | ERROR | Missing comment for param "$form_state" at position 2
171 | ERROR | Missing comment for @return statement
173 | ERROR | Method name
| | "counter_api_ctools_export_ui::increment_value_form_submit" is
| | not in lowerCamel format, it must not contain underscores
--------------------------------------------------------------------------------

FILE: ..._temp/test_candidate/plugins/export_ui/counter_api_ctools_export_ui.inc
--------------------------------------------------------------------------------
FOUND 4 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
2 | ERROR | Missing file doc comment
49 | ERROR | Inline comments must start with a capital letter
49 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
99 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.
Source: http://ventral.org/pareview - PAReview.sh online service

@ethanwl, you can feel free to ingore the coding standard regarding exported code.

We have some open issues with ctools and also some in drupalcs in regards to best solve this problem.

counter_api.module:69 and similar places:
watchdog('action', 'Incremented/decremented counter %counter_name', array('%counter_name' => check_plain($context['counter_name'])));

Here call to check_plain() is redudant, since using '%' placeholders is already checking for plain text.

Thanks for comparing to several modules.

How does this compare to http://drupal.org/project/sampler ?

Agreed there's not much duplication there - I ask mostly because others will ask and it's important to have a good answer for them. It would be great if you could add a wiki comparing all these at http://groups.drupal.org/similar-module-review and then link to that from your project page.

manual review of 6.x-1.x branch:

• counter_api_ctools_export_ui.class.php line 31 and 125, unexpected period at end of line.
• counter_api.install, line 96 ''columns' => array('pid' => 'pid'),': There is no field named 'pid' in the specified table, these should probably be 'cid'?
• admin/settings/actions: In the dropdown list the last item (generated by your module) shows up as "..." and trying to select it and press "Create" returns an error.
• counter_api_block.admin.inc: You have some empty functions at the end of the file, please remove them if not used.

Get a review bonus and we'll get back to you sooner.

Fixed typo.

Review of the 6.x-1.x branch:

• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

You can ignore the coding standard issues for class/method names.

manual review:

• "@param mixed $object": is this variable really mixet, or is it "object"? If it is mixed you should describe what types it can be.
• "$insert_sql = "INSERT INTO {counter_api_ticks} (cid, delta) values (%d, %d)";": you should use drupal_write_record() here.
• "TODO: Enter file description here.": Shouldn't be too hard to fill in a sentence, right?
• counter_api_block_theme(): you don't need to document the parameters of a hook implementation, see http://drupal.org/node/1354#hookimpl
• counter_api_block_get_config(): white space errors after "=>" and "=" in that function.
• "$block_name = "Counter: ";": all user facing text should run through t() for translation. And don't forget to use a placeholder.
• _counter_api_block_block_save(): "Returns the 'save' $op info for hook_block().": this function does not return anything?
• _counter_api_block_block_save(): "// Update block information in the block table.": comment is wrong, this is not what the code does.
• _counter_api_block_format_title(): admin_title and 'counter_name' are not sanitized, right? This can lead to XSS vulnerabilities.
• "$block['subject'] = $counter->description;" same here, XSS vulnerable I guess.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

Removed pending projects section: major one, Backbone, now has a few potential routes to full project-hood.