Commerce Nodecopy

Hello. I spent a Review of your module.
ventral.org/pareview is not work now and I did it manualy.

• commerce_nodecopy.install and commerce_nodecopy.rules_defaults.inc: @file is missing (Drupal Docs)
• commerce_nodecopy.rules.inc
• Line 59: use a space between the closing parenthesis and the open bracket
  if (is_array($line_item->{$field_name})){
• Line 75: missing space after comma
  $node = node_load($nid,null,true);
• If you write TRUE of FALSE, you must use capital characters.
• Comments in lines must end on dot.
• commerce_nodecopy.install: if this file is empty, maybe remove him?

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. 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.

The following git branches do not match the release branch pattern, you should remove/rename them. See http://drupal.org/node/1015226

<br>
  remotes/origin/7.x-1.x-dev<br>

Review of the master branch:

• README.txt is missing, see the guidelines for in-project documentation.
• ./commerce_nodecopy.rules.inc: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  
  

  <br>
  function commerce_nodeinfo_line_item_process($line_item) {<br>
  

• Drupal Code Sniffer has found some code style issues (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.

</code></p><code>
<p>FILE: ...w/sites/all/modules/pareview_temp/test_candidate/commerce_nodecopy.info<br>
--------------------------------------------------------------------------------<br>
FOUND 3 ERROR(S) AFFECTING 2 LINE(S)<br>
--------------------------------------------------------------------------------<br>
 7 | ERROR | It's only necessary to declare files[] if they declare a class or<br>
   |       | interface.<br>
 8 | ERROR | Files must end in a single new line character<br>
 8 | ERROR | It's only necessary to declare files[] if they declare a class or<br>
   |       | interface.<br>
--------------------------------------------------------------------------------</p>
<p>FILE: ...ites/all/modules/pareview_temp/test_candidate/commerce_nodecopy.install<br>
--------------------------------------------------------------------------------<br>
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)<br>
--------------------------------------------------------------------------------<br>
 1 | ERROR | Missing file doc comment<br>
--------------------------------------------------------------------------------</p>
<p>FILE: ...sites/all/modules/pareview_temp/test_candidate/commerce_nodecopy.module<br>
--------------------------------------------------------------------------------<br>
FOUND 8 ERROR(S) AND 1 WARNING(S) AFFECTING 9 LINE(S)<br>
--------------------------------------------------------------------------------<br>
 11 | ERROR   | Whitespace found at end of line<br>
 14 | ERROR   | Whitespace found at end of line<br>
 19 | ERROR   | There must be no blank line following an inline comment<br>
 24 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 25 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 26 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 27 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 28 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 34 | WARNING | A comma should follow the last multiline array item. Found: )<br>
--------------------------------------------------------------------------------</p>
<p>FILE: ...es/all/modules/pareview_temp/test_candidate/commerce_nodecopy.rules.inc<br>
--------------------------------------------------------------------------------<br>
FOUND 35 ERROR(S) AND 2 WARNING(S) AFFECTING 31 LINE(S)<br>
--------------------------------------------------------------------------------<br>
  4 | ERROR   | Whitespace found at end of line<br>
  8 | WARNING | Format should be "* Implements hook_foo()." or "Implements<br>
    |         | hook_foo_BAR_ID_bar() for xyz_bar()."<br>
  9 | ERROR   | Whitespace found at end of line<br>
 16 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 17 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 18 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 19 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 20 | ERROR   | Array indentation error, expected 10 spaces but found 12<br>
 21 | ERROR   | Array indentation error, expected 14 spaces but found 16<br>
 22 | ERROR   | Array indentation error, expected 14 spaces but found 16<br>
 23 | ERROR   | Array indentation error, expected 10 spaces but found 12<br>
 24 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 25 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 26 | ERROR   | Array indentation error, expected 10 spaces but found 12<br>
 27 | ERROR   | Array indentation error, expected 6 spaces but found 8<br>
 31 | ERROR   | More than 2 empty lines are not allowed<br>
 35 | ERROR   | Missing function doc comment<br>
 40 | ERROR   | Whitespace found at end of line<br>
 41 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or<br>
    |         | question marks<br>
 42 | ERROR   | AS keyword must be lowercase; expected "as" but found "AS"<br>
 43 | WARNING | Line exceeds 80 characters; contains 84 characters<br>
 43 | ERROR   | No space before comment text; expected "// YES!" but found<br>
    |         | "//YES!"<br>
 43 | ERROR   | Comments may not appear after statements.<br>
 46 | ERROR   | No space before comment text; expected "//<br>
    |         | dpm($wrapper-&gt;commerce_product-&gt;{$field_name}-&gt;raw());" but<br>
    |         | found<br>
    |         | "//dpm($wrapper-&gt;commerce_product-&gt;{$field_name}-&gt;raw());"<br>
 47 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or<br>
    |         | question marks<br>
 49 | ERROR   | Comments may not appear after statements.<br>
 50 | ERROR   | A cast statement must be followed by a single space<br>
 52 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or<br>
    |         | question marks<br>
 52 | ERROR   | Comments may not appear after statements.<br>
 53 | ERROR   | AS keyword must be lowercase; expected "as" but found "AS"<br>
 54 | ERROR   | A cast statement must be followed by a single space<br>
 59 | ERROR   | Expected "if (...) {\n"; found "if (...){\n"<br>
 75 | ERROR   | No space found after comma in function call<br>
 75 | ERROR   | No space found after comma in function call<br>
 75 | ERROR   | TRUE, FALSE and NULL must be uppercase; expected "NULL" but<br>
    |         | found "null"<br>
 75 | ERROR   | TRUE, FALSE and NULL must be uppercase; expected "TRUE" but<br>
    |         | found "true"<br>
 99 | ERROR   | Files must end in a single new line character<br>
--------------------------------------------------------------------------------</p>

FILE: ...dules/pareview_temp/test_candidate/commerce_nodecopy.rules_defaults.inc<br>
--------------------------------------------------------------------------------<br>
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)<br>
--------------------------------------------------------------------------------<br>
  2 | ERROR | Missing file doc comment<br>
 26 | ERROR | Files must end in a single new line character<br>
--------------------------------------------------------------------------------<br>

Source: http://ventral.org/pareview - PAReview.sh online service

manual review:

• "module_load_include('inc', 'clone', 'clone.pages');": clone module? why is this line needed?
• commerce_nodecopy_save(): I think that function should live in the module file as it might be useful for others.
• "module_invoke_all('commerce_nodecopy_presave', $node, $original_node);": if your module provides hooks you should document them in a *.api.php file.

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

just to get a little more picky :P

• readme, line 1: "git status" ?
• You could provide a readme in the master branch telling people to have a look at version specific branches
• your module depends on node_reference, add it to .info

I've tested it's functionality with commerce kickstart and it works good
But the usage instructions could be a little better.

I don't think there are any relevant security aspects..

So, also from my side this can be fixed.

As said in #8 I agree with klausi that this is ready-to-go.

Thanks for your contribution, farald! Welcome to the community of project contributors on drupal.org.

Don't forget to have a look at the non-blocking issues noted above.
It also seems to me that you're not correctly authenticated to git (have a look at the git instructions page on your project page for more info).

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

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.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.