WhiteJazz

Hi Alyx Vance,

Sorry, this process is often slow. We have a huge backlog of reviews and too few people doing reviews. Anyone can review projects in this queue, so I'd encourage you to seek out reviewers who may be interested in this. You may consider joining the Design 4 Drupal group and asking people there to come here and review your work:

http://groups.drupal.org/drupal-design

* Just the .info file is committed.
* Info file is not correct. http://drupal.org/node/542202

I'm trying to review your project, maybe I was not checking the right branch.

Yes, it's a theme, I was pasting a wrong link.

Back to needs review.

There's only an .info file in the master branch, which is what you get if you just follow the git clone instructions on the project page. There's no need to use the master branch, so that's not a problem beyond making it a little more difficult to review. The full git instructions explain how to get a different branch with git, e.g. the 7.x-1.x branch, which has all the files in it.

If this is an updated version of someone else's code, have you contacted them about it? Is the older version already on drupal.org?

The original is not on Drupal.org, though it is a Drupal theme. It's here: http://demo.roopletheme.com/whitejazz/

It's fully GPL, so licensing is at least not a problem. It would still be better if everyone worked together on Drupal.org, if that's an option.

This doesn't feel quite right - it would prevent adding the script to aggregation.

        <script type="text/javascript" src="<?php print $GLOBALS['base_url'] ."/"; print $directory; 

/js/suckerfish.js">

?>

I suggest looking for how to do that in template.php with drupal_add_js so it can be aggregated.

I tried mailing roopletheme as well to get their input on how to handle this. For example, some of the images might not be GPL. We shouldn't wait more than 2 weeks (and less could be OK since others have contacted them).

OK, seems RTBC to me.

• changelog.txt should be CHANGELOG.txt
• jquery.pngFix.js: appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms. The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.
• suckerfish.js: indentation should be 2 spaces per level, as in PHP files
• info file: remove "version", this is added by drupal.org packaging automatically
• All files and function should have proper doxygen doc blocks, see http://drupal.org/node/1354#files
• "} else if" should be "elseif" and be on a new line
• style.css: indentation errors, always use 2 spaces. See http://drupal.org/node/302199 . Same for ie.css
• Run coder to check your code style: http://drupal.org/project/coder

Review of the 7.x-1.x 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/theme-settings.php:
   +3: [minor] @file description should be on the following line
   +87: [minor] There should be no trailing spaces
  
  sites/all/modules/pareview_temp/test_candidate/template.php:
   +3: [minor] @file description should be on the following line
   +18: [minor] There should be no trailing spaces
   +21: [minor] There should be no trailing spaces
   +24: [minor] There should be no trailing spaces
   +26: [minor] There should be no trailing spaces
   +44: [minor] There should be no trailing spaces
   +59: [minor] There should be no trailing spaces
   +60: [minor] There should be no trailing spaces
   +68: [minor] There should be no trailing spaces
  
  Status Messages:
   Coder found 1 projects, 6 files, 11 minor warnings, 0 warnings were flagged to be ignored
  

• README.txt is missing, see the guidelines for in-project documentation.
• Remove "version" from the info file, it will be added by drupal.org packaging automatically.
• Remove "project" from the info file, it will be added by drupal.org packaging automatically.
• ./template.php: all functions should have doxygen doc blocks, see http://drupal.org/node/1354#functions
  

  
  function whitejazz_preprocess_block(&$variables) {
  --
  
  function whitejazz_process_html(&$variables) {
  

• Assignments should hava a space before and after the operator, see http://drupal.org/node/318#operators
  

  ./js/suckerfish.js:3:  for (var i=0; i<sfEls.length; i++) {
  ./js/suckerfish.js:4:    sfEls[i].onmouseover=function() {
  ./js/suckerfish.js:5:      this.className+=" sfhover";
  ./js/suckerfish.js:7:    sfEls[i].onmouseout=function() {
  ./js/suckerfish.js:8:      this.className=this.className.replace(new RegExp(" sfhover\\b"), "");
  

This automated report was generated with PAReview.sh, your friendly project application review script. Please report any bugs to klausi.

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

• Remove "version" from the info file, it will be added by drupal.org packaging automatically.
• Remove "project" from the info file, it will be added by drupal.org packaging automatically.
• Assignments should have a space before and after the operator, see http://drupal.org/node/318#operators
  

  ./js/suckerfish.js:3:  for (var i=0; i < sfEls.length; i++) {
  ./js/suckerfish.js:8:      this.className=this.className.replace(new RegExp(" sfhover\\b"), "");
  

This automated report was generated with PAReview.sh, your friendly project application review script. Please report any bugs to klausi.

I don't insist on a README.txt if you absolutely don't want it. Sorry for being so pedantic, but it is important to reach some level of quality here. Other people will better understand your code if it complies to standards, so it is easier for them to contribute patches for example.

Doc blocks on functions that implement hooks should look like this: http://drupal.org/node/1354#hookimpl

But that are just minor details, overall I think this is RTBC.

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

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 minde: Commit messages - providing history and credit and Release naming conventions.