Views record count

Welcome with your application.

Why not do this as a patch of Views? Why is a module needed? Have you raised an issue on this in the issue queue of Views?

So, should we close this application?

So, what is the status of this application, postponed?

Don't forget to set the status to "needs review" if you want to get a review.

I ran your code through PAReview, it came back with the following results:

http://ventral.org/pareview/httpgitdrupalorgsandboxankitchauhan1427124git

Please fix the problems listed by PAReview.

As for a manual review of the module, I installed the module on a fresh Drupal 6.24 install on localhost and used Devel generate to make some dummy content and comments. I then setup a view to display a list of content, and another view to setup a list of comments. Both were configured to have full paging (and I see that is where the settings live for this module). I then played around with the 'Display Record Count' and 'Choose Region for Record Count Message' (by the way, there is a spelling error for the word 'record').

First, I would recommend that the 'Display Record Count' and 'Choose Region for Record Count Message' should have default values selected, i.e. the 'Display Record Count' should default to 'No' and the 'Choose Region for Record Count Message' should default to header or footer.

I created a Page display for each of the views with a path of /content and /comments to see how this would look on a page. The displayed record count worked as expected.

However, I noticed that if I setup some overridden values on the page display and revisit the page, the page uses the default settings for 'Display Record Count' and 'Choose Region for Record Count Message', it doesn't use the overridden values. I setup some extra overridden values on 'Items per page' and 'Fields' just to make sure it wasn't views that was bugging out, which it wasn't. So it looks like the module needs to pay attention to overridden values and not grab the view's default display settings.

I did not test for the following situation, but it deserves a test before this module could be released.... what about other themes? (I just tested with Garland) What if a theme doesn't have a header/footer region (I know most themes would, but just in case a theme doesn't have those regions)? Perhaps the regions radio button list should be dynamically generated, that way users could select whatever region they would like.

Other than the items mentioned, this is a nice simple module and it does exactly what the name says it does.

Great. But, still, this project is too short to approve you as git vetted user. But you could still apply for have this promoted as a single project. Also you should document your functions. The comments does not say so much at this point. How about the issue that was in comment #11 about overridden values?

manual review:

• project page is too short, see http://drupal.org/node/997024
• I agree with Misc, this project is a bit short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project.

Otherwise RTBC to me for a single projet promote. Removing the review bonus tag, if there are still changes required you can add it again if you have done another 3 reviews of other projects.

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

That minor issues by automated review tools should not block a manual review.

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

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

  
  FILE: ...ites/all/modules/pareview_temp/test_candidate/views_record_count.module
  --------------------------------------------------------------------------------
  FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
  --------------------------------------------------------------------------------
   58 | ERROR | Missing parameter type at position 1
   61 | ERROR | Data type of return value is missing
  --------------------------------------------------------------------------------
  

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.

manual review:

• "format_plural($my_total, '@count record', '@count records') . ' found.": the word "found" should also be included in the translation.
• views_record_count_form_alter(): why do you need a switch() here if you check only one case? Use if() instead.
• Project page is still too short.

Otherwise this looks still RTBC to me for a single project promotion. Please don't remove the "single project promote" tag.

Also you did not review other project applications, so removing review bonus tag (please read #1410826: [META] Review bonus again).
What guidance do you need?

reviewing module

klausi,

I think the comment in #21 was with regards to not understanding your comment in #19 (i.e. I suspect a language issue).

ankitchauhan,
The comment in #19 was for Jalandhar ... but fix the items in #22. :)

reviewing project

Removing PAReview: review bonus tag.

This is reserved for people doing deep-dive 'manual reviews' of other projects, not simply commenting on singular issues. As directed before, please review #1410826: [META] Review bonus.

The project Views Record Count has been promoted to a full project.

Now that this experimental project has been promoted, you'll need to update the URL of your remote repository or reclone it.

git remote set-url origin ankitchauhan@git.drupal.org:project/views_record_count.git