[D7] twentyeleven

From pareview: http://ventral.org/pareview/httpgitdrupalorgsandboxbadri1397702git

These are all very simple fixes.

I also recommend putting templates in a templates/ folder, to separate them from the template.php and .info file, for readability.

You can fix the "Inline control structures are not allowed" errors by putting the if-else 'logic' in template.php and rendering the result with drupal_attributes.

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


FILE: .../sites/all/modules/pareview_temp/test_candidate/comment-wrapper.tpl.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
36 | ERROR | File doc comments must be followed by a blank line.
--------------------------------------------------------------------------------

FILE: ...pareview/sites/all/modules/pareview_temp/test_candidate/comment.tpl.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AND 3 WARNING(S) AFFECTING 4 LINE(S)
--------------------------------------------------------------------------------
9 | WARNING | Line exceeds 80 characters; contains 84 characters
30 | WARNING | Line exceeds 80 characters; contains 82 characters
36 | WARNING | Line exceeds 80 characters; contains 82 characters
58 | ERROR | File doc comments must be followed by a blank line.
--------------------------------------------------------------------------------

FILE: ...eview/sites/all/modules/pareview_temp/test_candidate/node--blog.tpl.php
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
78 | ERROR | File doc comments must be followed by a blank line.
87 | ERROR | Inline control structures are not allowed
--------------------------------------------------------------------------------

FILE: ...-7-pareview/sites/all/modules/pareview_temp/test_candidate/node.tpl.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
78 | ERROR | File doc comments must be followed by a blank line.
--------------------------------------------------------------------------------

FILE: ...-7-pareview/sites/all/modules/pareview_temp/test_candidate/page.tpl.php
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
5 | ERROR | Missing file doc comment
42 | ERROR | Inline control structures are not allowed
43 | ERROR | Inline control structures are not allowed
--------------------------------------------------------------------------------

FILE: ...pal-7-pareview/sites/all/modules/pareview_temp/test_candidate/style.css
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
519 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

FILE: ...-7-pareview/sites/all/modules/pareview_temp/test_candidate/template.php
--------------------------------------------------------------------------------
FOUND 14 ERROR(S) AFFECTING 14 LINE(S)
--------------------------------------------------------------------------------
2 | ERROR | Missing file doc comment
3 | ERROR | Missing function doc comment
5 | ERROR | Concat operator must be surrounded by spaces
7 | ERROR | Whitespace found at end of line
9 | ERROR | Array indentation error, expected 6 spaces but found 8
10 | ERROR | Array indentation error, expected 6 spaces but found 8
11 | ERROR | Array indentation error, expected 6 spaces but found 8
12 | ERROR | Array indentation error, expected 6 spaces but found 8
13 | ERROR | Array indentation error, expected 6 spaces but found 8
14 | ERROR | Array indentation error, expected 6 spaces but found 8
27 | ERROR | Missing function doc comment
34 | ERROR | Missing function doc comment
36 | ERROR | Line indented incorrectly; expected 2 spaces, found 4
39 | ERROR | Line indented incorrectly; expected 2 spaces, found 4
--------------------------------------------------------------------------------

The functional 'problems' I have with the theme:

• Too few regions. Regions are cheap and very useful.
• Too aggressive CSS reset. You override the font-family of all <pre> elements =(
• No logo.png in the theme folder. If you add one, it'll be displayed by default. Another option is overriding html.tpl.php and not showing a logo. Another option is preprocessing for it in the theme to not show it.

I like it though. I'm keeping it as base theme. Nice HTML5 + Aria.

Still some imperfections: http://ventral.org/pareview/httpgitdrupalorgsandboxbadri1397702git

The missing file doc comments is the most important. You can take it from Drupal's default page.tpl.php if the variables are identical. I suggest you add which regions are available.

There's also still the missing logo or empty logo html. It's not in html.tpl.php like I suggested before. It's in your page.tpl.php. If your theme shouldn't have a logo, remove the html from page.tpl.php (okay) or preprocess it out (better).

Hi lakshminp, I ran the code review again and it flagged the color codes, they mention that they prefer lowercase color codes here: http://drupal.org/node/302199

But I think the main issue I noticed is the lack of tabs. I installed your theme, and had no way to edit my content once I had created it other than going to the administration area. Your page.tpl.php should probably have a $tabs variable printed somewhere to allow users to edit the content.

Still these things need to be fixed:

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 7.x-1.x branch:

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

FILE: ...pal-7-pareview/sites/all/modules/pareview_temp/test_candidate/style.css
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
419 | ERROR | CSS colours must be defined in lowercase; expected #1982d1 but
| | found #1982D1
--------------------------------------------------------------------------------

FILE: .../modules/pareview_temp/test_candidate/templates/comment-wrapper.tpl.php
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
5 | WARNING | Line exceeds 80 characters; contains 81 characters
--------------------------------------------------------------------------------

FILE: ...ites/all/modules/pareview_temp/test_candidate/templates/comment.tpl.php
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 3 WARNING(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
9 | WARNING | Line exceeds 80 characters; contains 84 characters
30 | WARNING | Line exceeds 80 characters; contains 82 characters
36 | WARNING | Line exceeds 80 characters; contains 82 characters
--------------------------------------------------------------------------------

FILE: ...w/sites/all/modules/pareview_temp/test_candidate/templates/page.tpl.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
142 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

Here are things that needs to be fixed:

• Subject field in comment form overflows out of the box https://skitch.com/ilari/8ragc/subject
• Comment form buttons should be aligned side by side https://skitch.com/ilari/8ragu/buttons
• Comment form seems to be working correctly only when going to comment/reply/1#comment-form page.
• On a node page the header doesn't look right when having a sidebar https://skitch.com/ilari/8rags/nodepageheader
• Search functionality is broken. When entering a search term it doesn't search anything.
• The filtering tips page doesn't work when using this theme. The examples don't show as supposed https://skitch.com/ilari/8ra8k/filteringtips
• CSS stylesheet: You should use css reset in the beginning, you need to comment and structurize the code. Some links to aid in this CSS reset, 10 CSS best practices, Drupal CSS coding standards.

I noticed a few things
- My install shows a search button, which I don't think was intended: https://skitch.com/dudenhofer/886xj/review-site
- Not sure that it was intended to hide all other search form submit buttons. If I add a search block, it looks like this: https://skitch.com/dudenhofer/886tb/review-site
- At first I didn't think the image header upload worked, but it looks like it adds the image to the rotation of images. So it works, but there might be some confusion there.
- There is no $messages variable in the page template, so I currently see no system messages.

Hi,
Great choice, and nice implementation. The images in the header out of the box are real joy.
TwentyEleven is wonderful WordPress theme. Actually, I believe it is the WP default theme for fresh installations.
If so, we may consider it as WordPress equivalent to our Drupal Bartik.
As such, I would treat it with extra care and would try to be as close as possible to the original. "With great power, comes great responsibility..." (:

I think that you should mention in the project page the fact that this is/was the official default WordPress theme.

I installed tour theme in a dev environment ad also installed fresh WordPress site to get some impression of the original.

I faced few "need fix" thing:
wide images in posts are too large for the content area and overfloaw to the sidebar and break the visual page layout.

Full nodes titles are clickable as links (they shouldn't) .
If you decided to remove titles from page.tpl.php and move them to node.tpl.php you should distinguish between full node titles and linked teaser titles.

That bring us to the question of missing page titles.
Pages that are not full node pages appears without title . (taxonomy pages for example).
Also are missing required variables like :

<?php print render($page['help']); ?>
<?php print $feed_icons; ?>

You can install Style Guide module to examine your theme with many common styles.

It would be nice if you will include in the theme some more little things that can be found in the original WP like the comment icon, with comments count and the "search" text inside the search box.
You have already implemented HOOK_form_search_block_form_alter() to attach ID attribute to the submit button.
You can find more at How to Customize the Block Search Form

You have done quite good job on main details, It will be lovely if you can also stick with these little tiny style details of the original (and so popular) TwentyEleven theme.

Hi lakshminp,

There few style adjustments can be made to get closer to the original. For example the use of icons for comments, borders for images in node content and font sizes in few places. See the WP twentyeleven demo

But there is also major issue with HTML5/XHTML.
I have just noticed your page.tpl.php contains HTML5 tags. The Original WP twentyeleven theme is indeed HTML5 theme.
But Drupal 7 core DOCTYPE is still XHTML by default.

In order to produce valid and meaningful HTML5 theme you should, at least, implement HTML5 DOCTYPE via your template.php.
You will also need to modify more template files like html.tpl.php, node.tpl.php etc.

See Examples for HTML5 template files in Boron theme.
You can find more Drupal HTML5 inspiration in Adaptivetheme and Sasson.

The Images width:
I believe this is the WP theme way of dealing with wide images, width in percentage (style.css, line 61):

img.size-full,
img.size-large {
  max-width: 97.5%;
  width: auto;
  height: auto;
}

Another thing is the overflow of header images in narrower browser windows In not the case in original theme.
see attached screenshot images.

WP theme use percentage width for branding images( style.css line 530):

  #branding img {
     height: auto;
     margin-bottom: -7px;
     width: 100%;
  }

Hi lakshminp,

Well done for the HTML5 implementation.
Few more things:
1.
You need to soleve IE html5 problem, maybe by adding html5shiv to get your HTML5 theme to work in IE8 and below. If you will check in ie8 you will see the problem.
(These older IE versions need some JS help to use CSS with HTML5 elements).
I brought my copy of Drupal Twentyeleven to function in IE by adding theses lines in html.tpl.php (placed immidietly after the $scripts ):

 <?php print $scripts; ?>
  <!--[if lt IE 9]>
    <script src="//html5shiv.googlecode.com/svn/trunk/html5.js"></script>
  <![endif]-->

2.
At file comment.tpl.php you forgot to wrap strings in t() functions.

3.
The same with "Search" string inside the file twentyeleven.js - see Translating strings in JavaScript.

4.
In line 49 at field--taxonomy_term_reference.tpl.php
You print the $content_attributes in a way that seperate the the class attribute from its value.
Instead of

<div class=<?php print $content_attributes; ?>"taglinks">

I belive it should be:

<div class="taglinks" <?php print $content_attributes; ?>>

5.
I had some difficulties with managing the header images.
a. When I uploaded standard proportions image it displayed with its full height. (see attached screenshot).
b. You supplied mechanism to upload new photos to header but I have not seen any UI to remove certain photos from header.

6.
Layout on narrow screen width:
The original twentyeleven function very well in narrow viewports. It seems that the sidebar sinks below main content. The drupal implementation doesn't function this way and on narrow screen size and images from main content overlap/overflow into sidebar.
see attached images

7.
Strange thing: only in chrome - searchbox look strange. Work fine in IE(with html5shiv js) and Firefox - see sttached image.

7.5
The comments icon positioned, fixed, in top right corner of the screen (It hides behind toolbar, I have noticed only when no tool bar, as anonymous with add comments permissions )

8.
The theme is missing some typography styles for text in content.
No numbers on ordered lists,
No bullets on unordered lists
Html headers H's like H2, H3, etc. are lokk like ordinary text.
On comment #17 I recommended that you install and enable a wonderful tool for every themer, the Style Guide module. You can use it to see what I am saying.

Hi there mate!

Here is my manual review:

1) Your git clone code in this page's description is wrong. The correct is :
git clone --recursive --branch 7.x-1.x http://git.drupal.org/sandbox/badri/1397702.git twentyeleven
We need to clone the "7.x-1.x" branch, not the empty "master" ;)

2) Your theme is not finished. You must add more styling so your theme looks closer to the original theme (pagination, text boxes.. etc...). I suggest you to copy the exact same demo content from the Wordpress' Twenty Eleven demo site. Then make you drupal demo site look as close as possible to the original.

3) I get the following warning:

Warning: Creating default object from empty value in twentyeleven_preprocess_html() (line 117 of sites\all\themes\twentyeleven\template.php).

Note that im testing it on my localhost where i have enabled full error reporting. This wont be displayed on production servers. Here is a screenshot (i get this error on every single page).

4) In "style.css" line 492 you have declared "position: absolute;" followed by "top: 0.2em;" and "top: 0.2em;" which is not properly displayed in webkit browsers. The comment counter is stuck on the top right corner. Check the screenshot. I suggest you to use "display: inline-block", "float: left" or fix the html markup so you can properly display it.

5) In "style.css" line 34 you use percentage unit for the margin property. Avoid that. Try to use the same unit for all properties. Use "em" or "px" for margin and padding.

6) In "style.css" line 44 your gradient will work only on webkit and mozilla browsers. Have a look at Ultimate CSS Gradient Generator from ColorZilla for cross-browser compatible css gradients.

7) In "style.css" line 108 your gradient wont work because its overridden by the solid color on line 115. You must have the solid color in the first line, then add the gradients for backward comparability. Please read CSS coding standards and the paragraph about "CSS3 properties, vendor prefixes, progressive enhancement".

Currently your theme is not properly displayed on my site that i tested. I saw few screenshots from users that reviewed your theme before me, and what i have is different. I think its because you formatted your css with automated css formatter in your text editor and some properties moved or got their priority overridden. This is my homepage: Screenshot. As you can see the logo is misplaced, the titles doesn't have the same styling as the original theme, the pager is not styled correctly ... etc

I hope my review helps you improve your theme.
We all appreciate your work. Thanks!
- ANDiTKO

hi lakshminp,

here is manual review from me,
I think it's not a bad idea to give margin or padding from logo to site title, it's look so close.
and you should look the file I attached, I think that's need padding too, and the last I think this theme will looks better if the main content has margin the right side.

sorry for my bad english. :D

Looks like the previous two comments indicate that this needs some work.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

The git clone command is wrong, and your project defaults to the master branch which is empty. You should fix these if you want reviews!

		<a href="<?php print check_url($front_page); ?>">

Sanitization should be done in the preprocess. Though won't this already have been sanitized?

				$menu_name = variable_get('menu_main_links_source', 'main-menu');
				$main_menu_tree = menu_tree($menu_name);

This should be done in preprocess.

			<?php if ($search_box): ?>
				<div id="searchform">
					<?php print $search_box; ?>
				</div>
			<?php endif; ?>

I appreciate you're trying to do a straight port of the WP theme, but it still seems a bit weird to hardcode a block.

      <?php
      print render($item);
      if ($delta < (count($items) - 1)):
        print ",";
      endif;

Again, you're trying to match what WP does. But there are modules that do that. It would be better to suggest that your users use https://drupal.org/project/textformatter on their term fields rather than duplicate its work.

/**
 * Implements hook_form_alter().
 */
function twentyeleven_form_search_form_alter(&$form, &$form_state, $form_id) {

Function docblock is wrong.

Sounds good.

You should consider mentioning https://drupal.org/project/textformatter in the README, so users who want WP-style comma-separate terms can easily set them up.

Automatic review

The automatic review found some trivial problems, including one spelling error.
Please see: PAreview for details.

Manual review

This is said to be a port of Wordpress TwentyEleven,
which is licensed under GPL v2.
According to the license, any derivative must meet the following conditions:

You must cause the modified files to carry prominent notices stating that you changed the files and the date of any change.

The code for the Drupal version seems to be much modified from the WordPress version, however, I could not find any notice about what is changed, and how.
While it is OK to have third party code under GPL hosted on Drupal.org, the basic requirements that allows reuse of a project under GPL must be in place.

Documentation for seems to be lacking. For instance, a number of sample images is packaged with the theme, and the demo screenshot shows an image in some region. But just installing the module did not show an image anywhere and there was no obvious settings to activate it or change it.

> The code for the Drupal version seems to be much modified from the WordPress version, however, I could not find any notice about what is changed, and how.

I imagine the underlying code is completely different. There is a matter of the *design* being licensed though maybe?

> No. Very little guidance about how to configure the module is offered.

I'm not sure that's needed. All themes are configured in the same way, in the same part of the UI.

> Documentation for seems to be lacking. For instance, a number of sample images is packaged with the theme, and the demo screenshot shows am image in some region. But just installing the module did not show an image anywhere and there was no obvious settings to activate it.

I get an image shown when I enable the module.

> '#upload_location' => path_to_theme() . "/images/headers",

That shouldn't work. If it does, then it's a security risk!

> Moved the hardcoded search form to a search region.

Several artefacts of that remain -- in template.php and page.tpl.php.

div.tabs stretches across the whole page, and so if I put the navigation menu block in the RH sidebar, the bottom border of the tabs appears to the right and it looks a bit odd.

When I enabled the theme, the blocks that were in the footer in my old theme got put into the search region. That was a bit weird. You might want to review the procedure that the theme system uses when figuring out which regions to copy blocks into when enabling a theme for the first time -- it might be that there are hints you can give it (or it might merely be the order in which you declare regions?).

You might want to consider adding a Help region -- the core themes all have that.

IMHO, this theme is not documented well enough.

This is the markup produced by this theme on my site. In the URL to the image, the path is present, but the file name with the image is missing.

<img id="header-image" typeof="foaf:Image" src="http://example.com/subsite/" width="1000" height="288" alt="" title="" />

This is either a bug, or some setting is missing at my site (and I haven't a clue about what that setting is). Either way, this is a blocker.

Missing alt and title tags are also IMHO bad coding practice.

> '#upload_location' => path_to_theme() . "/images/headers",

You should never be able to upload files into your codebase! Files should go in the public:// directory.

Automatic review

The automatic review found some trivial problems.
Please see: PAreview for details.

Manual review

The description following the @file tah in html.tpl.php spans two lines. It needs to be rewritten to fit on a single line. Suggestion:

/**
 * @file
 * Implementation to display the basic html structure of a single Drupal page.

The Drupal version is based the WordPress version. The README.txt has the following notice:

TwentyEleven is a Drupal theme based on Wordpress' TwentyEleven theme (http://wordpress.org/themes/twentyeleven)

The Wordpress' TwentyEleven theme is licensed under GPL v2.

According to the Drupal.org policy on 3rd party code, it is permitted if the code is made available under GPLv2 and the original has to be modified to work with Drupal. This is clearly the case with a port such as this one. The project also contains a number of images from the original library, including eight images in the images/haeders/ directory. Again, images available under GPLv2 is permitted (ibid.).

However, according to the license, any derivative must meet the following conditions:

You must cause the modified files to carry prominent notices stating that you changed the files and the date of any change.

In the current distribution, I could not find any notice about what is changed, and how.

For this theme to satisfy the licensing requirement, the notice in README.txt saying that it is based upon the Wordpress' TwentyEleven must be expanded to explain that the files has been changed, and how, as well as the date of any change,

The documentation is improved, and there is now a useful README.txt. However, the lines are too long, and there is not enough whitespace to make it easy to read, and there are a number of minor inaccuracies (for instance,the "Search form" is referred to as the "Search block".

Below is a suggestion for how the README.txt can be improved.

INTRODUCTION
------------

TwentyEleven is a Drupal theme based on Wordpress' TwentyEleven theme
(http://wordpress.org/themes/twentyeleven).  See the section CHANGES
below for description of how it is changed.

TwentyEleven for Drupal has the following features:

 * Front page with choice of One column/ Two column layout.
 * Drop down menus.
 * Configurable sidebar position (left or right).
 * Fully responsive theme.
 * Footer with 3 block regions.
 * Masthead images. A number of sample images are provided. Custom
   images can be uploaded.

INSTALLATION
------------

Install as you would normally install a contributed drupal theme.
See: https://drupal.org/documentation/install/modules-themes/modules-7
for further information.

CONFIGURATION
-------------

To achieve the same desired look and feel as Wordpress's TwentyEleven,
it is preferred to place the Search form in the Search region.

To configure the position of the serach block, visit
/admin/structure/block and use the pull-down menu to place the Search
form in the Search region.

The following configuration items are available for TwentyEleven. They
can be configured by visiting admin/appearance/settings/twentyeleven.

* Header Images: They must be one of jpg, bmp, gif or png formats and
  1000 x 288 pixels.  Users can choose to display a random header
  image or a specific fixed header image.
* Sidebar Position: can be chosen to be placed either left or right.

CHANGES
-------

TwentyEleven is a Drupal theme based on Wordpress' TwentyEleven theme
(http://wordpress.org/themes/twentyeleven).

The starting point for this Drupal version of the theme was 
WordPress' TwentyEleven version 1.X.

In order to create this Drupal theme, the following changes were done:

[HERE, EXPLAIN WHAT YOU'VE CHANGED.]

All header images and icons that accompany this Drupal theme is copied
frow the WordPress theme without changes.

MAINTAINERS
-----------

* Lakshmi Narasimhan (lakshminp) - https://drupal.org/user/1022778
* Thomas Lattimore (tlattimore) - https://drupal.org/user/624754

Manual review summary

This sentence (from README.txt) makes it appear as the theme is derived from or is an adaption of the Wordpress' TwentyEleven theme:

TwentyEleven is a Drupal theme based on Wordpress' TwentyEleven theme

The same with this sentence (from the project page):

Twenty Eleven is based on wordpress' Twenty Eleven theme.

If the actual PHP-code, HTML-markup, JavaScript and CSS is not based on the WordPress theme, and only the images are copied, you should instead say something like this:

TwentyEleven is a Drupal theme inspired by Wordpress' TwentyEleven theme
(http://wordpress.org/themes/twentyeleven). All header images and icons that
accompany this Drupal theme is copied from the WordPress theme without
changes, and is used here under GPLv2.  Apart from the images and icons,
everything else in the Drupal version is created from scratch.

When you're reusing works created by others, it is necessary to make it clear to users what is copied, and what is not copied, and that you have the necessary permissions to use the works that are copied,

Currently this is how it looks in the system module's html.tpl.php. I copied the comments from there only. Should I still change this?

Yup. The coding standard says one line. The system module's html.tpl.php breaks the coding standard, but the system module is not under review here.

To me, the theme now looks like it is ready for being promoted.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/1397718

Project 2: https://www.drupal.org/node/1313026

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.

I've got two weeks more experience reviewing, so I am going back in my own tracks. I am afraid I have to withdraw my previous conclusion on this. Below is a revised review.

Automated Review

PAReview reported some issues. However, please note that these are not substantial and fixing all issues is not a requirement for getting through the application process.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party code/content

Yes: Follows the guidelines for 3rd party code.

The module reuses media content from the WordPress TwentyEleven theme (http://wordpress.org/themes/twentyeleven). All header images and icons that accompany this Drupal theme is copied from the WordPress theme without changes, and is used under GPLv2. Apart from the images and icons, everything else in the Drupal version is created from scratch. The use of the media content is acknowledged in README.txt, making reuse under GPLv2 permissible.

README.txt

Yes: Follows the guidelines for in-project documentation.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

No: Re guidelines for writing secure code. See below for the list of security issues.

Coding style & Drupal API usage

My notes:

• Do not call time(). Use REQUEST_TIME instead.
• Use drupal_basename(). PHP's basename() does not properly support streams or filenames beginning with a non-US-ASCII character.
• (*) The following: $vars['comment_string'] = t('!author on !permalink said:', array('!username' => $vars['author'], '!permalink' => $vars['permalink'])); uses !username as placeholder for !author.
• (*) Several placeholders look like they are not suffiscently santitzed. For instance, anonymous comments allows the name of the author to be entered as a plain textfield. Using placeholder @author is more robust. For example: $vars['submitted'] = t('!username on', array('!username' => $vars['author'])); should be $vars['submitted'] = t('@username on', array('@username' => $vars['author']));.

The starred items (*) are fairly big issues and warrant going back to Needs Work. The rest of the comments in the code walkthrough are recommendations.

Please don't remove the PAReview: security security tag, we keep that for statistics and to show examples of security problems.

I'll get back to you ASAP with a full the review, but since this can take some time to get resolved, I just want to point out another stumbling block:

The way I now read the guidelines for 3rd party libraries and content on Drupal.org, two conditions have to be fulfilled for third party content to be included in the repo:

• It must be available under GPL V2+. (You are OK in this one.)
• You need to obtain explicit approval by drupal.org administrators or delete the 3rd party content. (This one remains.)

I've set status to Postponed. Please follow one of the routes below, and set back to "Needs review" when one of the routes has been completed.

1. Get approval from the Drupal.org webmasters. Post a request to include the photographs and any other material from the WordPress distribution to the Webmasters issue queue Reopen with "Needs review" when you've obtained the required permission.
2. Remove the images. Replace all 3rd party material from the project, and replace it with photograhs, etc. that you've created yourself. Reopen with "Needs review" when you've pushed a version where all 3rd party content is removed.

Are creative commons licensed images allowed for the header image?

The policy says no third party content can be included unless explicit permission is granted by the webmasters. The way I understand the policy, it is the same whether it is CC or GPL V2 or MIT or BSD or OFL.

I am really sorry. I don't agree with this policy myself. But as a reviewer, I have to go by the book.

Automated Review

PAReview reported some issues. These are not substantial and fixing all issues is not a requirement for getting through the application process.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party code/content

Yes: Follows the guidelines for 3rd party code.

README.txt

Yes: Follows the guidelines for in-project documentation.

Since you've now replaced this with your own images, you need to remove the the following from README.txt:

All header images and icons that accompany this Drupal theme is copied
from the WordPress theme without changes, and is used here under GPLv2.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes. (Guidelines for writing secure code).

Coding style & Drupal API usage

• It is pointless to run a string that only consists of a placeholder through t(): Replace:
  

  $vars['permalink'] = l(t('@datetime', array('@datetime' => $vars['created'])), $uri['path'], $uri['options']);
  

  with:

  $vars['permalink'] = l(check_plain($vars['created']), $uri['path'], $uri['options']);
  

• This is rather awkward, and you need to maintain the same list og image extensions in two places:
  

  $images = file_scan_directory($dir, "/\\.jpg$|\\.JPG$|\\.jpeg*|\\.JPEG*|\\.gif$|\\.GIF$|\\.png$|\\.PNG$/");
  $custom_headers_path = 'public://twentyeleven_headers';
  if (file_prepare_directory($custom_headers_path)) {
    $images = array_merge($images, file_scan_directory($custom_headers_path, "/\\.jpg$|\\.JPG$|\\.jpeg*|\\.JPEG*|\\.gif$|\\.GIF$|\\.png$|\\.PNG$/"));
  }
  

  This does the same, defines the list in one place, and is case insensitive:

  $image_extensions = '/\\.' . implode('|\\.', array('jpg$', 'jpeg*', 'gif$', 'png$')) . '/i';
  $images = file_scan_directory($dir, $image_extensions);
  $custom_headers_path = 'public://twentyeleven_headers';
  if (file_prepare_directory($custom_headers_path)) {
    images = array_merge($images, file_scan_directory($custom_headers_path, $image_extensions));
  }
  

Please don't remove the PAReview: security security tag, we keep that for statistics and to show examples of security problems.

Changing status back to RTBC.

Automated Review

A few issues have cropped up in the automated review again: http://pareview.sh/pareview/httpgitdrupalorgprojecttwentyelevengit

•  style.css: Unit of measure px is redundant at line 137, 138, 140, 492, 493, 494
• js file: Duplicated jQuery selector (at line 4 & 5 & 11)

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt

Yes: Follows the guidelines for in-project documentation and the README.txt Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes. If "no", list security issues identified.

Coding style & Drupal API usage

• (+) js doesn't follow the standard Drupal method to attach to a page. Please review https://www.drupal.org/node/756722#behaviors
• (*) Potential issue: I'm not familiar with the original wordpress theme, is reset.css from the original wordpress theme? Or from somewhere else? If it is from somewhere else, appropriate attribution should be granted.
• (+) screenshot.png appears to still use the previous graphics, which is confusing to someone using this theme.
• (+) html.tpl.php: There is no locally stored library for the HTTP link. (at line 43) Pulling a file from subversion seems like a bad idea for performance. Can this be switched for a CDN?
• (+) page.tpl.php: This file uses tabs rather than the standard two spaces for whitespace.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

Moving back to needs work to hear response on the reset.css. Otherwise, this has had several reviews at this point and is a fairly solid theme.

I'm not sure what specifically was addressed from #1397718-57: [D7] twentyeleven but my question about reset.css was answered.

Here's what I saw in a quick glance that wasn't addressed. There might be other stuff as well.

1. screenshot still seems to be an issue.
2. duplicate js selector is still an issue.
3. context in js isn't used. e.g. $('input[name="search_block_form"]', context)

However, none of these are blockers... and since this was already RTBCed previously and just waiting on an answer to reset.css...

Thanks for your contribution, lakshminp!

I updated your account so you can 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 stay 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.