[D7] Views Table Rowspan

Automated report from pareview.sh

FILE: ...view/pareview_temp/views/plugins/views_plugin_display_table_rowspan.inc
--------------------------------------------------------------------------------
FOUND 14 ERRORS AFFECTING 6 LINES
--------------------------------------------------------------------------------
8 | ERROR | Class name must begin with a capital letter
8 | ERROR | Class name must use UpperCamel naming without underscores
15 | ERROR | Method name
| | "views_plugin_display_table_rowspan::option_definition" is not in
| | lowerCamel format, it must not contain underscores
15 | ERROR | Visibility must be declared on method "option_definition"
25 | ERROR | Method name "views_plugin_display_table_rowspan::options_form" is
| | not in lowerCamel format, it must not contain underscores
25 | ERROR | Visibility must be declared on method "options_form"
41 | ERROR | Method name
| | "views_plugin_display_table_rowspan::render_grouping_sets" is not
| | in lowerCamel format, it must not contain underscores
41 | ERROR | Visibility must be declared on method "render_grouping_sets"
67 | ERROR | Type hint "array" missing for $sets
67 | ERROR | Method name
| | "views_plugin_display_table_rowspan::get_colspan_rows" is not in
| | lowerCamel format, it must not contain underscores
67 | ERROR | Visibility must be declared on method "get_colspan_rows"
98 | ERROR | Type hint "array" missing for $set
98 | ERROR | Method name
| | "views_plugin_display_table_rowspan::_get_deepest_row" is not in
| | lowerCamel format, it must not contain underscores
98 | ERROR | Visibility must be declared on method "_get_deepest_row"
--------------------------------------------------------------------------------

Please fix this issue and then check your module on pareview.sh .

Whenever above issue is done i will check again.

Thanks for Contribution.

Please make sure that your commits are on the 7.x-1.x branch, that that is the default branch (In Sandbox Project > Version Control) and that Master is deleted.

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.

This looks very similar to a project I created, https://www.drupal.org/project/views_grouped_table.

If they are indeed providing the same or similar features, I'd be very happy to make you a co-maintainer.

Your git clone command

git clone --branch 7.x-1.x howto@git.drupal.org:sandbox/howto/2300969.git

Is a personal git clone command, and should be updated to the general purpose command.

Look good, I'd like this module available in full project.

@howto
Your module looks good to me as well. As I reviewed your code and also installed on my local machine.
Suggestion
Please add hook_help function to .module file.

@howto
You should really join @joachim in his existing project https://www.drupal.org/project/views_grouped_table.

Create an issue in that project issue queue with the title "Applying for commit access" and describe your motivation. Read Joining forces with others and co-maintaining projects and related docs for more information.

If this is an option, then this issue should be closed or at least postponed, so project application reviewers won't spend time to review this sandbox.

Review of the 7.x-1.x branch

[D7] Views Table Rowspan

Functional testing:

• Installed and enabled the module
• Went to add a shared category field to both basic page and article
• Created a taxonomy vocab to use for them (with video, science, tech, culture)
• Added 6 articles and 6 basic pages and selected a category for each of them
• Added a new view with
  • Show content of type 'All'
  • Display format 'Table Rowspan'
• Added the fields to display;
• Content type
• Content category (shared field)
• Content title
• Content post date
• Set page: style options to group field Nr. 1 to (content) Type
• Kept 'Merge rows in table' checked
• Saved the view and saw as shown in screenshot

PAReview:

• view/pareview_temp/views/plugins/views_plugin_display_table_rowspan.inc
• Line 8: views_plugin_display_table_rowspan must;
• Begin with a capital letter.
• UpperCamel naming without underscores.
• Be prefixed with the project name 'ViewsTableRowspan'.
• Line 15:
• function must be in lowerCamel format without underscores.
• Visibility must be declared on method "options_form".
• Line 41:
• method name must be in lowerCamel format without underscores.
• Visibility must be declared on method "render_grouping_sets".
• Line 67:
• Type hint "array" missing for $sets.
• Protected method name "get_colspan_rows" is not in lowerCamel format.
• Line 98:
• Type hint "array" missing for $sets.
• Protected method name "_get_deepest_row" is not in lowerCamel format.

Coder review (using the minor (most) option to see everything.

• sites/all/modules/contrib/views_table_rowspan/views_table_rowspan.module
• See https://www.drupal.org/node/1354#functions for the following;
• Line 11: Docblock should be immediately above views_table_rowspan_help
• function views_table_rowspan_help($path, $arg) {
• Line 17: The correct use of the string is 'e.g.,' (with a comma after it).
• However, for clarity, consider changing 'e.g.' to 'for example,'.
• $help .= ' . t('Create a !link (eg: a list of node).', array('!link' => l(t('new view'), 'admin/structure/views/add'))) . '
• Line 30: Docblock should be immediately above views_table_rowspan_views_api
• function views_table_rowspan_views_api() {
• Line 40: Docblock should be immediately above views_table_rowspan_theme_registry_alter
• function views_table_rowspan_theme_registry_alter(&$theme_registry) {
• Line 49: Docblock should be immediately above views_table_rowspan_proprocess
• function views_table_rowspan_proprocess(&$vars) {

@joris_lucius
About views/plugins/views_plugin_display_table_rowspan.inc: this class inherits Views native plugin declaration, so it is OK to be an exception from the coding standard rule.
As for results of testing, you should had set issue status to Needs work.

Git default branch is not set, see the documentation on setting a default branch.

Review of the 7.x-1.x branch (commit 967e76c):

• Remove "version" from the ./views_table_rowspan.info file, it will be added by drupal.org packaging automatically.
• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  FILE: /home/klausi/pareview_temp/css/views_table_rowspan.css
  --------------------------------------------------------------------------------
  FOUND 6 ERRORS AFFECTING 6 LINES
  --------------------------------------------------------------------------------
    2 | ERROR | Line indented incorrectly; expected 2 spaces, found 4
    4 | ERROR | Expected 1 space before opening brace of class definition; 0
      |       | found
    5 | ERROR | Line indented incorrectly; expected 2 spaces, found 4
    6 | ERROR | Line indented incorrectly; expected 2 spaces, found 4
    9 | ERROR | Expected 1 space before opening brace of class definition; 0
      |       | found
   10 | ERROR | Line indented incorrectly; expected 2 spaces, found 4
  --------------------------------------------------------------------------------
  

• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

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. You have to get a review bonus to get a review from me.

manual review:

1. please add the differences to the existing views_grouped_table module to the project page, so that user can make an educated decision which module to use in which case.
2. "function views_table_rowspan_proprocess" should be "function views_table_rowspan_preprocess"?

But that are not critical application blockers, otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to patrickd as he might have time to take a final look at this.

Hidiho, let's see if I can still do this :)

Default branch wasn't set, I set it to 7.x-1.x (https://www.drupal.org/node/2300969/edit/default-branch)

#1
#2
I'd recommend not to put link URLs into the t() function, this can cause complications with translations. Use @url placeholder instead.

#3version = 7.x-1.0-devdont't set the version in your .info file, when you create a release later the drupal.org packaging system will set it automatically

I did no functional testing, but this looks good enough to me regarding best practices.

Thanks for your contribution!

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.