[D7] Views breadcrumb

http://ventral.org/pareview/httpgitdrupalorgsandboxrakeshjames2049937git

@see drupal coding standards

FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 5 WARNING(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
5 | WARNING | Line exceeds 80 characters; contains 120 characters
11 | WARNING | Line exceeds 80 characters; contains 115 characters
13 | WARNING | Line exceeds 80 characters; contains 86 characters
26 | WARNING | Line exceeds 80 characters; contains 89 characters
27 | WARNING | Line exceeds 80 characters; contains 86 characters
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/views_breadcrumb.module
--------------------------------------------------------------------------------
FOUND 117 ERROR(S) AND 1 WARNING(S) AFFECTING 42 LINE(S)
--------------------------------------------------------------------------------
4 | ERROR | The second line in the file doc comment must be " * @file"
4 | ERROR | Expected 1 space between asterisk and tag; 0 found
4 | ERROR | Expected 1 space(s) before asterisk; 0 found
5 | WARNING | Line exceeds 80 characters; contains 89 characters
5 | ERROR | The third line in the file doc comment must contain a
| | description and must not be indented
5 | ERROR | Expected 1 space(s) before asterisk; 0 found
6 | ERROR | Expected 1 space(s) before asterisk; 0 found
6 | ERROR | Additional blank line found at the end of doc comment
7 | ERROR | The last line in the file doc comment must be " */"
7 | ERROR | Expected 1 space(s) before asterisk; 0 found
10 | ERROR | Expected 1 space(s) before asterisk; 0 found
10 | ERROR | Function comment short description must start with exactly one
| | space
11 | ERROR | Expected 1 space(s) before asterisk; 0 found
12 | ERROR | Expected 1 space after closing parenthesis; found 0
16 | ERROR | Expected 1 space before "=>"; 0 found
17 | ERROR | Expected 1 space before "=>"; 0 found
17 | ERROR | Expected 1 space after "=>"; 0 found
17 | ERROR | Whitespace found at end of line
more.....

Automated review shows errors [ https://drupal.org/sandbox/rakeshjames/2049937 ]. You might have corrected your coding errors, but no push to git was done.

Manual review: views_breadcrumb.module

line 17 in hook_block_info:
'pages' =>'BLOCK_VISIBILITY_LISTED',
this line is not correct,because:

• BLOCK_VISIBILITY_LISTED is a constant, don't use commas, beacuse it prefills the url list in "Show block on specific pages" with 'BLOCK_VISIBILITY_LISTED' as text
• also, for correct usage of hook_block_info parameters, check https://api.drupal.org/api/drupal/modules!block!block.api.php/function/h...

I assume you wanted something like

'visibility' => BLOCK_VISIBILITY_LISTED,
'pages' => "......",

Also I've added the block in the content part and keep getting lots of errors, more and more after every refresh. I've added a snapshot for you to check.

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.

Hello,

There still remains some issues with the automated review, mostly indenting issues.
Please see : http://ventral.org/pareview/httpgitdrupalorgsandboxrakeshjames2049937git

Ensure you are working in a version specific branch.
Although master branches are commonly used in the Git world, the Drupal community uses major version branches (e.g., 7.x-1.x, 6.x-1.x) instead, since master does not indicate what Drupal version the project is compatible to.
The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

Refer to the application checklist

On line 35 of your module, I believe drupal_set_message still requires the use of the t() function for text being displayed to the user.

Please tell me , is there any way to check my code myself in ventral.org?

You can just go to ventral.org and use http://git.drupal.org/sandbox/rakeshjames/2049937.git for your project.
There still seems to be some issues reported, all indention errors.

For line 35 I think you should use:

However, for links enclosed in translatable text you should use t() and embed the HTML anchor tag directly in the translated string. For example:

t('Visit the <a href="@url">settings</a> page', array('@url' => url('admin')));

see function l()

assuming ownership for review

Overall I could see this as being a very helpful module. I did notice a few things. Please see below.

You need to delete your master branch. When I did a git clone to pull your source, I pulled the master branch by default. See details here.
views_breadcrumb.module lines 33 & 38: These fetches will work but what happens if the query returns more than one row. Should subsequent rows not be included in the body of the block?
You might also consider using placeholders in your calls to db_query instead of injecting the value into the query directly. It probably isn't as important here because you are not injecting stuff inputted from the user, but it's a good habit to get into. See here.
Finally, the project page and README.txt file could contain more helpful information.

See more details below. I checked this module using the checklist found here.

1. Basic application checks
1.1 Project application contains a working git clone command and a link to the project page.
1.2 Project does not appear to be a duplicate.
1.3 User does not have multiple applications.

2. Basic repository checks
2.1 Repository contains code
2.2 Repository contains a version specific branch, but the master branch still exists. I would merge all code from the master branch to the 7.x-1.x branch (if not done already) and remove the master branch. See here.
2.3 Project contains less than five functions and less than 120 lines of code. Minimum code requirements have not been met.

3. Security review
3.1 Some SQL queries have variables injected into them directly as opposed to using placeholders. See here.

4. Licensing checks
4.1 Repository does not contain a LICENSE.txt file
4.2 Repository does not contain any third party code.

5. Documentation checks
5.1 Project page does not contain detailed information. Where do the breadcrumbs originate? How can you specify what the text of the breadcrumb is and where it points?
5.2 Repository contains a README.txt file, and the README.txt file explains how to install the module and how to configure. It might be helpful to explain what the module requires, what the module does, what configuration options are available, etc.
5.3 Code contains comments for the file and the functions, but could use more inline comments.

6. Coding Standard and style
6.1 I ran a review on ventral.org http://ventral.org/pareview. The review returned an unused variable error, and the review mentioned that the master branch still exists. I couldn't find anything major.

7. API and best practices review
7.1 Project makes correct use of drupal apis.

I am sorry but I do not quite understand the logic behind this module. After review the code I see that it provides a block that grabs menu structure and build a breadcrumbs from it. And this block works only in case it is placed on the page that is views page.

@rakesh.gectcr Could you please correct me if I am wrong? Also please advise what is the use case for this module to be used?

PS I completely agree with #13 regarding using placeholders in db_query. Please avoid injecting values in the db_query directly.

@rakesh.gectcr why would you need this custom module instead of standard drupal functionality or any breadcrumbs modules (like https://drupal.org/project/custom_breadcrumbs) or similar?

Thanks for contributing to Drupal! Can you provide some more information about the questions below?

Possible Duplication

It seems that Custom Breadcrumbs provides very similar functionality - and it claims to support Views out of the box. Could you describe how your module differs?

Also Path Breadcrumbs seems similar. Could you describe how your module differs?

We prefer collaboration over competition, therefore we want to prevent having duplicating modules on drupal.org. If the differences between these modules are not too fundamental for patching the existing one, we would love to see you joining forces and concentrate all power on enhancing one module. (If the existing module is abandoned, please think about taking it over).

If that fails for whatever reason please get back to us and set this back to "needs review".

Code too short

This project is too 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 for you.

----
Top Shelf Modules - Crafted, Curated, Contributed.

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.