Cogito Theme

I ran this through the Coder module and it comes out pretty clean. You might want to consider testing it using the "minor" setting and tidying up a few of the trailing space and tab indent issues that occur on that. I will review the actual code shortly.

form.inc.php
Line 83: missing space after comma
  element_set_attributes($element, array('id', 'name','#return_value' => 'value'));
Line 164: There should be no trailing spaces
  
Line 292: Use an indent of 2 spaces, with no tabs
         case 'checkbox':
Line 293: Use an indent of 2 spaces, with no tabs
         case 'radio':
Line 381: missing space after comma
  element_set_attributes($element, array('id', 'name','#return_value' => 'value'));

template.php
Line 18: There should be no trailing spaces
  global $theme; 
Line 20: There should be no trailing spaces
  
Line 22: There should be no trailing spaces
  
Line 36: There should be no trailing spaces
  
Line 47: There should be no trailing spaces
        $favicon =is_file( $path_to_child . 'favicon.ico' ) ? $path_to_child . 'favicon.ico' : $path_to_cogito . 'favicon.ico'; 
Line 53: There should be no trailing spaces
    
Line 79: There should be no trailing spaces
  //variables to it 
Line 89: There should be no trailing spaces
  
Line 91: There should be no trailing spaces
  
Line 96: There should be no trailing spaces
  $icon57 = is_file( $root_d7 . $path_to_child . 'apple-57x57.png' ) ? $path_to_child . 'apple-57x57.png' : $path_to_cogito . 'apple-57x57.png'; 
Line 97: There should be no trailing spaces
  $icon72 = is_file( $root_d7 . $path_to_child . 'apple-72x72.png' ) ? $path_to_child . 'apple-72x72.png' : $path_to_cogito . 'apple-72x72.png'; 
Line 98: There should be no trailing spaces
  $icon114 =is_file( $root_d7 . $path_to_child . 'apple-114x114.png' )?$path_to_child . 'apple-114x114.png':$path_to_cogito . 'apple-114x114.png'; 
Line 99: There should be no trailing spaces
  
Line 133: There should be no trailing spaces
    if ( !is_file( $root_d7 . "/" . $path_to_child . "/logo.png" ) && 
Line 137: There should be no trailing spaces
 
Line 146: There should be no trailing spaces
  
Line 162: There should be no trailing spaces
  
Line 179: There should be no trailing spaces
    //four is a nice small number that will still show something      
Line 186: There should be no trailing spaces
  
Line 191: Use an indent of 2 spaces, with no tabs
   $variables['cogito_header'] = theme('cogito_header', $variables);  
Line 191: There should be no trailing spaces
   $variables['cogito_header'] = theme('cogito_header', $variables);  
Line 192: There should be no trailing spaces
  
Line 211: There should be no trailing spaces
  
Line 268: There should be no trailing spaces
    'status' => t('Status message'), 
Line 269: There should be no trailing spaces
    'error' => t('Error message'), 
Line 272: There should be no trailing spaces
  
Line 277: There should be no trailing spaces
  
Line 359: There should be no trailing spaces
        'class' => array('pager-first'), 
Line 365: There should be no trailing spaces
        'class' => array('pager-previous'), 
Line 374: There should be no trailing spaces
          'class' => array('pager-ellipsis'), 
Line 382: There should be no trailing spaces
            'class' => array('pager-item'), 
Line 388: There should be no trailing spaces
            'class' => array('current'), 
Line 394: There should be no trailing spaces
            'class' => array('pager-item'), 
Line 401: There should be no trailing spaces
          'class' => array('pager-ellipsis'), 
Line 407: There should be no trailing spaces
    
Line 411: There should be no trailing spaces
        'class' => array('pager-next'), 
Line 417: There should be no trailing spaces
        'class' => array('pager-last'), 
Line 422: There should be no trailing spaces
      'items' => $items, 

Hi,

There are many errors of style, see http://ventral.org/pareview/httpgitdrupalorgsandboxraychaser1494826git

• In the README line does not exceeds 80 characters,
• Errors on comments formatting,
• See how to indent the code,
• ...

First correct all these errors so that the code is readable for reviewers,
Thanks.

Please do not assign your own applications, only the current reviewer should do this

Hi,

This my first review; hope i am not doing something wrong.

First thank you for sharing this theme. Here are what (little) problems i saw in base theme. I know it is a base theme; maybe what i see as problem is what expected; i just wanted to share.

There are sizing and positioning problems about text boxes and submit buttons. Search box and button are not in same row and overflow out of the header div.

Tested in Firefox 11.0

and

Chrome 16.0

In WP version button is above the text box.

In footer text box and button in same row but their heights are different.

There are similar different height problems in child theme.

Given that this is a starter theme just like Zen or Omega, I think it's in great shape to be released as a starter theme. It contains all of the default Foundation styles as they are applied to that framework and the STARTER THEME can quickly be replicated to start creating a solid theme. Starter or base themes are just that, base themes, they are not intended to be placed on a site and expected to make it look pretty. Zen's description for example states:

Zen is a powerful, yet simple, HTML5 starting theme with a responsive, mobile-first grid design. If you are building your own standards-compliant theme, you will find it much easier to start with Zen than to start with Garland or Stark

"Starting theme", "..much easier to start with Zen..."

I'd like to propose that this theme gets published as a starter theme based on the Foundation framework.

+1 for approving this theme.

The Foundation framework on which this theme is based isn't really production-ready out of the box like, e.g. Bootstrap. You're meant to take it and customize it which is why it's been developed as a starter theme with only minimal attention to design. As mentioned previously here, there are already many, many such themes on d.o.

Your project page is not very detailed, please have a look at the tips for a great project page, you may also use HTML-tags for better structure.

I can understand your desperation about code formatting nitpicking, anyway please describe what you actually changed in your commit messages ;-) (also see Commit messages - providing history and credit)

If your altering a single form and there's nothing special about it, rather use hook_form_FORM_ID_alter() instead of hook_form_alter()
In cogito_form_alter() you should also make the 'Search' strings translatable by encapsulating them with t().

cogito_preprocess_region: don't implement hooks if they don't do anything
cogito_preprocess_menu_link: even if your planning to do something in here, comment the complete function out

As I got no idea about theme best practices I'll let another admin have a second look ;)

We have currently exceeded the proposed project application thresholds, so this is on hold for me for now. Get a review bonus and I will review/approve this right away.

Sorry for the delay.

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards). See attachment.
• 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. You have to get a review bonus to get a review from me.

manual review:

1. "$root_d7 = getcwd();": no need for that, just use the constant DRUPAL_ROOT.
2. "if (user_access('Access administration menu')) {": I think that permission start lower case, no?
3. "$reaction = context_get_plugin('reaction', 'block');": so your theme depends on the context module? Please add that to the installation instructions.
4. why is cogito_menu_local_tasks() in form.inc.php?

But that are no application blockers, so ...

Thanks for your contribution, raychaser!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

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.

Thanks to the dedicated reviewer(s) as well.