TB Nucleus

Sandbox link: http://drupal.org/sandbox/themebrain/1233310

Hi themebrain,

Please fix the following issues, then move this back to "needs review" status:

• The directory structure is confusing. The .info file in the root directory has no theme associated, and should be deleted, replaces with a README.txt. Each theme directory should be moved to the top level directory, rather than nested inside the "nucleus" directory. In short, the root directory should have 3 items: README.txt, nucleus (which is currently in nucleus/nucleus) and nucleus_starter
• All CSS files have the wrong line endings. Everything in Drupal uses Unix-style \n line endings, but you're using either \r or \r\n. The non-CSS files don't seem to have this problem.
• I see a few coding standards issues. Please run this through the Coder module on the minor setting and fix everything it shows.
• All functions declared by a theme need to have names that begin with the name of the theme. Everything in custom_functions.inc is prefixed with tb_, which is not safe to do outside a module or theme named "tb". Same thing in nucleus.js and popup_extend_class.js.
• jquery.matchHeights.min.js is 3rd party code, which can't be hosted on Drupal.org. You should instead provide download instructions for this file in a README.txt file.

Don't add LICENSE.txt. That gets added automatically.

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:

• README.txt is missing, see the guidelines for in-project documentation.
• ./nucleus/nucleus/inc/theme_function_overrides.inc: all functions should have doxygen doc blocks, see http://drupal.org/node/1354#functions
  

  
  function nucleus_qt_accordion($vars) {
  

• ./nucleus/nucleus/inc/custom_functions.inc: The description for the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions
  

  277- *  all regions
  279- *  all active blocks
  304- *  result of nucleus_get_regions_blocks_list
  330- *  specific key for each "block style - extend class" setting
  332- *  type of setting 'global', 'region', 'block'
  334- *  is this block is showed (TRUE/FALSE)?
  

• Bad line endings were found, always use unix style terminators. See http://drupal.org/coding-standards#indenting
  

  ./nucleus/nucleus/css/ie.css:                                           ASCII C program text, with CR line terminators
  ./nucleus/nucleus/css/ie7.css:                                          ASCII text, with CR line terminators
  ./nucleus/nucleus_starter/css/views.css:                                ASCII C program text, with CR line terminators
  

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

manual review:

• put your theme in the root folder of the repository, like here http://drupalcode.org/project/omega.git/tree/refs/heads/7.x-3.x
• nucleus/nucleus_starter/js/jquery.matchHeights.min.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.

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

• Comments should be on a separate line before the code line, see http://drupal.org/node/1354#inline
  

  ./nucleus/tpl/page.tpl.php:127:  <!-- //HEADER -->
  ./nucleus/tpl/page.tpl.php:138:    <!-- //MAIN NAV -->
  ./nucleus/tpl/page.tpl.php:150:    <!-- //SLIDESHOW -->
  ./nucleus/tpl/page.tpl.php:162:    <!-- //HELP -->
  ./nucleus/tpl/page.tpl.php:172:    <!-- //PANEL FIRST -->
  ./nucleus/tpl/page.tpl.php:205:          <!-- //MAIN CONTENT -->
  ./nucleus/tpl/page.tpl.php:214:            <!-- //SIDEBAR FIRST -->
  ./nucleus/tpl/page.tpl.php:225:          <!-- //SIDEBAR SECOND -->
  ./nucleus/tpl/page.tpl.php:247:    <!-- //BREADCRUMB -->
  ./nucleus/tpl/page.tpl.php:257:    <!-- //PANEL SECOND -->
  ./nucleus/tpl/page.tpl.php:269:    <!-- //FOOTER -->
  ./nucleus_starter/tpl/page.tpl.php:127:  <!-- //HEADER -->
  ./nucleus_starter/tpl/page.tpl.php:138:    <!-- //MAIN NAV -->
  ./nucleus_starter/tpl/page.tpl.php:150:    <!-- //SLIDESHOW -->
  ./nucleus_starter/tpl/page.tpl.php:162:    <!-- //HELP -->
  ./nucleus_starter/tpl/page.tpl.php:172:    <!-- //PANEL FIRST -->
  ./nucleus_starter/tpl/page.tpl.php:205:          <!-- //MAIN CONTENT -->
  ./nucleus_starter/tpl/page.tpl.php:214:            <!-- //SIDEBAR FIRST -->
  ./nucleus_starter/tpl/page.tpl.php:225:          <!-- //SIDEBAR SECOND -->
  ./nucleus_starter/tpl/page.tpl.php:246:    <!-- //BREADCRUMB -->
  ./nucleus_starter/tpl/page.tpl.php:256:    <!-- //PANEL SECOND -->
  ./nucleus_starter/tpl/page.tpl.php:268:    <!-- //FOOTER -->
  

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.

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

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.
• All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
  

  ./README.txt ./nucleus/nucleus.info ./nucleus/css/theme-settings.css ./nucleus/css/ie.css ./nucleus/css/messages.css ./nucleus/css/base.css ./nucleus/css/ie7.css ./nucleus/css/grid/fluid/grid-12.css ./nucleus/css/grid/fluid/grid-24-opera.css ./nucleus/css/grid/fluid/grid-24.css ./nucleus/css/grid/fluid/grid-16.css ./nucleus/css/grid/fluid/grid-16-opera.css ./nucleus/css/grid/fluid/grid-12-opera.css ./nucleus/tpl/fieldset.tpl.php ./nucleus/tpl/admin/nucleus-extend-class-form-groups.tpl.php ./nucleus/tpl/admin/nucleus-extend-class-form-classes.tpl.php ./nucleus/tpl/admin/nucleus-extend-class.tpl.php ./nucleus/tpl/node.tpl.php ./nucleus/tpl/user-profile-category.tpl.php ./nucleus/tpl/aggregator-summary-items.tpl.php ./nucleus/tpl/field--taxonomy-term-reference.tpl.php ./nucleus/tpl/comment.tpl.php ./nucleus/tpl/panel.tpl.php ./nucleus/tpl/html.tpl.php ./nucleus/tpl/aggregator-feed-source.tpl.php ./nucleus/tpl/aggregator-item.tpl.php ./nucleus/tpl/page.tpl.php ./nucleus/tpl/comment-wrapper.tpl.php ./nucleus/tpl/block.tpl.php ./nucleus/js/nucleus.js ./nucleus/js/popup_extend_class.js ./nucleus/inc/preprocess_functions.inc ./nucleus/inc/custom_functions.inc ./nucleus/inc/alters.inc ./nucleus/theme-settings.php ./nucleus/template.php ./nucleus_starter/css/forms.css ./nucleus_starter/css/panel-panes.css ./nucleus_starter/css/quicktabs/simply.css ./nucleus_starter/css/print.css ./nucleus_starter/css/forum.css ./nucleus_starter/css/page.css ./nucleus_starter/css/html-elements.css ./nucleus_starter/css/typo.css ./nucleus_starter/css/fields.css ./nucleus_starter/css/views.css ./nucleus_starter/css/articles.css ./nucleus_starter/css/navigation.css ./nucleus_starter/css/superfish/simply.css ./nucleus_starter/css/comments.css ./nucleus_starter/css/blocks.css ./nucleus_starter/css/fonts.css ./nucleus_starter/tpl/page.tpl.php ./nucleus_starter/nucleus_starter.info ./nucleus_starter/theme-settings.php
  

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.

Please refer to http://drupal.org/node/539608 regarding issue priority (only one day had passed; priority timelines always refer to the last review done).

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.)

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

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):
  

  
  FILE: ...l/modules/pareview_temp/test_candidate/nucleus/inc/custom_functions.inc
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   42 | ERROR | Whitespace found at end of line
  --------------------------------------------------------------------------------
  
  
  FILE: ...ites/all/modules/pareview_temp/test_candidate/nucleus/tpl/block.tpl.php
  --------------------------------------------------------------------------------
  FOUND 3 ERROR(S) AFFECTING 3 LINE(S)
  --------------------------------------------------------------------------------
   15 | ERROR | Whitespace found at end of line
   18 | ERROR | Whitespace found at end of line
   19 | ERROR | Whitespace found at end of line
  --------------------------------------------------------------------------------
  
  
  FILE: ...es/all/modules/pareview_temp/test_candidate/nucleus/tpl/comment.tpl.php
  --------------------------------------------------------------------------------
  FOUND 3 ERROR(S) AFFECTING 3 LINE(S)
  --------------------------------------------------------------------------------
    9 | ERROR | Whitespace found at end of line
   30 | ERROR | Whitespace found at end of line
   37 | ERROR | Whitespace found at end of line
  --------------------------------------------------------------------------------
  
  
  FILE: ..._temp/test_candidate/nucleus/tpl/field--taxonomy-term-reference.tpl.php
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   5 | ERROR | Whitespace found at end of line
  --------------------------------------------------------------------------------
  
  
  FILE: ...sites/all/modules/pareview_temp/test_candidate/nucleus/tpl/html.tpl.php
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   12 | ERROR | Whitespace found at end of line
  --------------------------------------------------------------------------------
  
  
  FILE: ...modules/pareview_temp/test_candidate/nucleus_starter/theme-settings.php
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   10 | ERROR | Expected 1 space between the closing parenthesis and the opening
      |       | brace; found 2
  --------------------------------------------------------------------------------
  

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.

Manual additions:

• There are still files in your master branch. You should replace them as described in http://drupal.org/node/1127732 at step 5.
• I am not too familiar with theme project folder structures. But shouldn't be your base theme in the project root folder and only their sub-themes in sub folders? Just to mention and pls. correct me if I'm wrong.

hth, dave

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

• 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. Go and review some other project applications, so we can get back to yours sooner.

Manual review:

• nucleus_form_search_form_alter(): why do you need that function in your theme? Please add a comment why you alter that form.
• theme-settings.php: why do you require alters.inc and theme_function_overrides.inc there?
• Unfortunately there was a bug in pareview.sh that causes confusion regarding new lines at the end of files. There should be no extra empty line at the end of a file. Just make sure that the file ends in '\n'.
• "'#title' => t($value . " @width", array("@width" => t('width'))),": why do you need @width here? $value should be passed in with a place holder.
• "'#title' => t($key),": why do you translate the key here? And are you sure that you don't need to sanitize $key, i.e. is it user supplies data?
• "Impliments hook_form_system_theme_settings_alter().": typo, should be "Implements ..."
• nucleus_form_system_theme_settings_alter(): do not use drupal_add_js() and drupal_add_css() here, use #attached on the form: http://api.drupal.org/api/drupal/developer--topics--forms_api_reference....

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

Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):

FILE: ...al-7-pareview/sites/all/modules/pareview_temp/test_candidate/README.txt
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
48 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

FILE: ...iew/sites/all/modules/pareview_temp/test_candidate/nucleus/nucleus.info
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
90 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

FILE: ...dules/pareview_temp/test_candidate/nucleus_starter/nucleus_starter.info
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
140 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

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

• why does the starter kit contain actual code and CSS settings? I think it should just be a stub with some README.txt files that explain which overrides belong in which folder.
• still an "Impliments" in nucleus_starter / theme-settings.php
• "'#title' => check_plain(t('Region:') . ' ' . $region['region_title']),": you should use the @ placeholder for t() which runs check_plain() for you.
• Also in other places, do not concatenate variables to translated strings, use placeholders to insert them.
• nucleus_get_active_blocks_by_theme(): use line breaks for such long queries, e.g. http://drupal.org/node/310075#the_big_picture
• "$form['#attached']['js']['var nucleusStyleSupportCounter = '...": javascript settings should be added with the "data" key, see http://api.drupal.org/api/drupal/developer--topics--forms_api_reference.... . Also in many other places.
• project page is too short. What are the differences to other base themes? See also http://drupal.org/node/997024
• "$options[$i] = ($i == 1) ? t($i . ' grid') : t($i . ' grids');": again, do not concatenate variables with translatable strings, use placeholders. Check all you calls to t() in your code.
• "$layout_width = $layout_width_int > 100 ? ($layout_width_int . "px") : ($layout_width_int ? ($layout_width_int . "%") : "960px");": This line is too long and hard to follow, I would split it up to proper if statements over multiple lines.

Get a review bonus and we will come back to your application sooner.

Update product description

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

• 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.

manual review:

• Take a look at the CSS coding standards: http://drupal.org/node/302199 . There are many empty classes in your starterkit, I guess that is intentional and you don't need to fix that.
• nucleus.js: I think you should use Drupal.behaviors here (I'm not a javscript expert) http://drupal.org/node/756722 . Same for popup_extend_class.js

Otherwise I think this is nearly ready. I can't say much about all the theme functions and I don't know where text sanitization is necessary and where not.

Looks good to me. Removing review bonus tag.

Thanks for your contribution! 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 mind: Commit messages - providing history and credit and Release naming conventions.

Hi, I'm using TB_Rave on top of nucleus, and I'm having an issue with comment html tags.
Absolutely no tags whatsoever are being passed through, despite me allowing them at the drupal and editor level. Everything works fine for normal content, but comments are completely devoid of line breaks (most frustrating) and styling.
I'm posting here because this is also the case on all other TB themes that run on top of nucleus.

Thanks for the cool themes though, everything is GREAT otherwise!