[D7] Site Info

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 Rishi,

Automated Review
⊠ Git default branch is not set, see the documentation on setting a default branch https://www.drupal.org/node/1659588

Individual user account
☑ Follows the guidelines for individual user accounts.

No duplication
☑ Does not cause module duplication.

Master Branch
☑ Yes

Installation / Usage
⊠ Private function have missing docs https://www.drupal.org/coding-standards/docs#functions

For consistency, your git clone project name should be siteinfo instead of site_info

Secondly, some minor code formatting suggestions per http://pareview.sh/pareview/httpgitdrupalorgsandboxrahulseth2422343git review:

FILE: /var/www/drupal-7-pareview/pareview_temp/siteinfo.module
---------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
---------------------------------------------------------------------------
78 | ERROR | [ ] Return type missing for @return tag in function comment
143 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 4
145 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 4
147 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 4
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------

the minor coding standard errors and the clone command are surely not application blockers, anything else that you found or should this be RTBC instead?

I see no blockers. Manual review looks good.

As per #6, I've changed the git clone project name from site_info to siteinfo

This looks like a fairly basic hello-world sort of module, and doesn't add much that is not already available, in core even.
It gets some existing info from variables, then demonstrates you can put it on the screen with theme('table',...)

It's the sort of stuff you should be able to put together in the first day of following instructions to write your first Drupal 7 module. It's OK as a proof-of-concept ... but not useful as a stand-alone module to add to the existing throng. And though it's not wrong code it's not exactly a broad or a substantial demonstration of Drupal APIs.

Fully half or the items listed are already provided out-of-the-box from core on the /admin/reports/status page!
Other listings (number of content types, number of roles) are already easily accessible through the UI already, or if you wanted to report on them in one place, the thing to do would be to add an entry in the core "Status report" by implementing hook_requirements()

Pre-existing modules like this have a long history

• devel does heaps of this already
• system_status does this, and collates info for graphing and multi-server management
• devel_info seems to do much of this also, but I've not tried it because it doesn't add much that's not already easily available
• sitedoc did this years ago, and did so many times more than this! (and was extensible with hooks)
• site_audit which probes your site settings usefully (and does back-end support, AND can be extended with security review)

To add any value, a module in this space should do things that are not already done, and probably try to provide a back-end, such as a machine-readable feed or api, or something that extends or improves on the cli drush status. Or even better, EXTEND the existing ones using their APIs to provide proper synergy.

It's an incredibly softball sort of module to use as a project application. I'd much rather you built something useful for everyone to demonstrate your coding abilities.

Automated Review

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

Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
FILE: /var/www/drupal-7-pareview/pareview_temp/siteinfo.module
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
78 | ERROR | Return type missing for @return tag in function comment
----------------------------------------------------------------------

Time: 103ms; Memory: 7Mb

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.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

No: Causes module duplication and/or fragmentation.
Do not try to duplicate functionality because "you don't really like how it's done there". That only adds clutter. Work to improve an existing module rather than introduce yet another random module, as that leads to confusion and frustration for the development, support and end user communities.
The same feature is provided by devel module and Drupal core. Others can be achieved using views module with better flexibility

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

No: Does not follow the guidelines for project length and complexity.
Agree with dman, sort of basic coding example.

Secure code

Yes: Meets the security requirements

Coding style & Drupal API usage

[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

1. On line no 100, t() function missing, similarly from line no 117 to 131.
2. (*) On line no 103, you are using entity_load(user), which loads all other info of users which you ain't using. You may use EntityFieldQuery instead to find the number of users. Also make sure you mention active or blocked users.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

Overall I feel you might add additional information provided by your module as a patch to devel or other similar modules.

@Rahul Seth,
Please specify the difference on the project page and also on the issue summary page so that it will help other reviewers.
Manual Review :

1. You can configure settings form link through .info file to make it directly accessible from module page.
2. siteinfo_details() : Return the render array to the block content so that it will easily be alterable at the theme level.Just distinguish the code slightly a bit and used the theme output for the admin/siteinfo callback.
3. Readme.txt is nice.

4. function siteinfo_permission() {
     return array(
       'access admin information' => array(
         'title' => t('Access admin information'),
         'description' => t('View site information.'),
         'restrict access' => TRUE,
       ),
     );
   }

   The permission name 'access site information' looks more user friendly than 'access admin information'

5. siteinfo_details() : Add some more comments above foreach loops of modules, roles and bundles,roles,....

Not Found any major security issue, major api problems,License issue.
Setting this to RTBC :)

I would recommend you, please help to review other project applications to get a
review bonus back. This will put you on the high priority list :)
Thanks to you for your hard work and patience.

P.S. : Note for me the pareview template was used earlier so not used here.

Added difference w/r other projects and review of other applications in review

So I think this is sufficiently different to the existing projects since the scope is a bit different.

manual review:

1. project page: "Known problems" should describe what the known problems within your module are, not what use case the module solves. I think you can just remove that section.
2. siteinfo_block_view(): do not call render() here, just return the render array on the block as is, Drupal core will render it later for you.
3. "t("@dr_ver", array('@dr_ver' => $drupal_version))": that translation is not useful and just creates overhead for translators in the translation UI. Just pint the version information as is? Same for most other dynamic values that you translate here, that does not make sense.
4. siteinfo_details(): I think you should not call theme() here and just return the render array, so it can be altered more easily by other modules.
5. "$header = array('Content Type', 'Node', 'Roles', 'Modules', 'Active users');": that values should run through t() for translation.
6. siteinfo_details(): wait, you are listing all users of the site here? That will not scale if I have 10.000 users in my database? I think it does not make sense to list all users here, maybe a counter per role instead?

But that are not critical application blockers, so ...

Thanks for your contribution, Rahul Seth!

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.