Domain Background

Marking "needs review" so reviewers will see it.

The waiting time for a project application review is currently approaching six weeks. Please consider shortening your wait time by contributing to the code review process. All it takes is basic module writing skills, plus it is a great way to add to your knowledge of Drupal.Please visit http://groups.drupal.org/code-review for details on how to participate.

* no README.txt
* Remove old CVS helpers ($Id) from all your files
* code style:

drupal_set_message('File has been uploaded to ' . $file->filepath );

there should be no space before ')'

Automated review (Please keep in mind that this is primarily a high level check that does not replace but, after all, eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.)

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.
Review of the master branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards):
  

  Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
  
  sites/all/modules/pareview_temp/test_candidate/domain_background.module:
   +4: [minor] @file description should be on the following line
   +6: [minor] There should be no trailing spaces
   +8: [minor] Comment should be read "Implements hook_foo()."
   +15: [minor] Comment should be read "Implements hook_foo()."
   +25: [minor] Comment should be read "Implements hook_foo()."
   +29: [minor] There should be no trailing spaces
   +52: [normal] The $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.
   +52: [critical] Potential problem: drupal_set_message() only accepts filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
   +61: [minor] There should be no trailing spaces
   +62: [minor] There should be no trailing spaces
   +65: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
   +67: [minor] There should be no trailing spaces
   +68: [minor] There should be no trailing spaces
   +84: [minor] There should be no trailing spaces
   +86: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
   +92: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
   +101: [minor] There should be no trailing spaces
   +103: [minor] There should be no trailing spaces
   +165: [minor] There should be no trailing spaces
   +167: [minor] There should be no trailing spaces
  
  Status Messages:
   Coder found 1 projects, 1 files, 1 critical warnings, 4 normal warnings, 15 minor warnings, 0 warnings were flagged to be ignored
  

• README.txt is missing, see the guidelines for in-project documentation.
• @file doc block is missing in the module file, see http://drupal.org/node/1354#files .
• Comments should be on a separate line before the code line, see http://drupal.org/node/1354#inline
  

  11:  return array('administer domain background'); // Todo: not implemented anywhere else yet
  

• All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
  

  ./readme.txt ./domain_background.module ./domain_background.info
  

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

It appears you are still 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.
Review of the master branch:

• README.txt is missing, see the guidelines for in-project documentation.

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

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

Switched back to needs review, so in-depth reviews won't be blocked by coding standart issues.

1. 'core' line in your .info file represents Drupal core, not your module.

2. For readability, please indent the options array in your $form item definitions.

Otherwise, it looks relatively clean ... could perhaps use another set of eyes on it. (Short and quick!)

As per the release naming conventions, your branch should be named 6.x-1.x instead of 6.x-1.0 (which follows the 'tag' naming convention).

As branches represent a chain of commits over time, we use the '1.x' pattern when labelling them ... the code in a 'branch' changes with each commit added to the branch. A tag, however, is more specific, in that it represents the code at a specific point in time ... thus the '1.0' pattern.

Because Drupal's packaging scripts assume these branch/tag naming patterns, it becomes fairly important to understand the difference. :)

Manual review

1. Suppose group of sites running on one Drupal installation and a single shared database and these sites are
   • asdf.com
   • one.asdf.com
   • two.asdf.com

   as domain_background_settings_form() is used to configure background, the values of variables (domain_background_path, domain_background_color, domain_background_position, ...., domain-background-filename) get override by the last saved configuration as these sites use single shared database and in hook_init() only last generated CSS will get included as value domain-background-filename also get override in last saved configuration.

2. time() get added in file name, on each configuration saved a new file get created. I think need to add code to remove unused files

Feel free to change the status if this is incorrect.

Sorry for the delay, but you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

There are still files other than README.txt in the master branch, make sure to remove them. See also step 5 in http://drupal.org/node/1127732
Review of the 6.x-1.x branch:

• README.txt is missing, see the guidelines for in-project documentation.
• Drupal Code Sniffer has found some issues with your code (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.

manual review:

1. domain_background_perm(): what does the todo here mean? The permission is used in hook_menu()?
2. "Implements of hook_perm()." should be "Implements hook_perm().". Also elsewhere, see http://drupal.org/node/1354#hookimpl
3. "t('File has been uploaded to ') . check_plain($file->filepath)": do not concatenate dynamic values directly into translatable strings, use placeholder instead. Also, if you use the "@" or "%" placeholder with t() you will not need check_plain() here as this is done automatically by t().
4. domain_background_settings_form(): the #options values should also tun through t() for translation.
5. you should implement hook_uninstall() to remove any variables that your module might have created.

This application is not fixed? See http://drupal.org/node/532400

Please reopen if you still want to work on this application.