[D7] Events Conferences Theme

Hi lexicon

I am getting error while going on "http://drupal.org/sandbox/lexicon/1960090.git" this page. (Page Not found)

And while taking git clone
git clone --branch lexi-ect-7.x-1.x lexicon@git.drupal.org:sandbox/lexicon/1960090.git
form above url it asking password.

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

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

Now one can access my theme without usin password

Hi Lexicon,

I downloaded your theme and tested it in a fresh install I got the following errors:

Notice: Undefined index: lexi_event in theme_get_setting() (line 1419 of /opt/projects/eventsconferences/drupal/includes/theme.inc).
Notice: Trying to get property of non-object in theme_get_setting() (line 1456 of /opt/projects/eventsconferences/drupal/includes/theme.inc).
Notice: Trying to get property of non-object in theme_get_setting() (line 1466 of /opt/projects/eventsconferences/drupal/includes/theme.inc).

I noticed also that many functionality is missing (slideshow, header ..), I found out the problem is:

• Your theme folder name is "events_conferences_theme"
• Your theme folder name is "events_conferences_theme.info"

and your theme name in the .info file is like this

name = lexi_event
description = Lexicon event theme 
package = Core
core = 7.x

To fix the problem rename your theme folder name and the .info file

Code review

• You used frequently the theme_get_setting inside page.tpl.php the best practice is to use it inside a preprocess function then just use the variables inside your .tpl.php files
• You need to separate content from code example :
  

        <li id ="banner2" > <img src="<?php print base_path() . drupal_get_path('theme', 'lexi_event') . "/" .                         theme_get_setting('image_2'); 

  print theme_get_setting('discription_2');

  print drupal_get_path_alias(theme_get_setting('url_2')); "

• I dont see why you need to hard code a slideshow while there is many good modules to do it like Flex Slider and Views Slideshow etc ..
• Use l() function better then using html tags to print links inside a .tpl.php file
• In the file lexi.js you mixed your cutom Javascript code with a library which need to be separated, libraries should go to /sites/all/libraries

At this stage this is the major issues that's need to be fixed

I removed you automated review comments from the issue summary.

Don't forget to add the "PAReview: review bonus" tag next time when you complete 3 reviews as indicated in http://drupal.org/node/1975228 , otherwise you won't show up on my high priority list.

Great looking theme!
A couple of small things:
- On settings page, "Copy Right" should be "Copyright"
- I'm getting a JS error: "Uncaught SyntaxError: Unexpected token + " from responsiveslides.min.js on line 85
- Is this a dependency? I doesn't seem to be listed as such
- line 580 of style.css, I might suggest changing the vertical position of the background-image of the "Read More" link to 3px instead of 6... currently, it's appearing very low
- Does the banner need to display at the top of the configuration pages? it's a little distracting.
... I know these are nit-picks, but I like this theme!

~R~

Hi,

Manual Review :

(1) page.tpl.php

1. wrong description/help text; seems you have used the files from Bartik Theme but not changed the text/word Bartik.
Example : Bartik's theme implementation to display a single Drupal page.

Should be
YOUR-THEME-NAME theme implementation to display a single Drupal page.

2. no need to force user to use non-clean URL's
Example : print drupal_get_path_alias(theme_get_setting('url_1', 'events_conferences_theme')); " />

(2) Spelling Errors

Example : line no - 132
'#title' => t('Registeration'),
Registeration should be Registration

(3) Remove Un-wanted/Commented Code

Example : css/style.css - line no : 229, 244-245

(4) seems there is no font-kit or thirdy-party/google fonts are added for "andale mono", "lucida console", monospace; so that will not take effect on the machines which doesn't have "andale mono", "lucida console", monospace; fonts installed on their machine.

Thanks,
Ujval

Hi Lexicon,

I downloaded your theme and tested it manually on my localhost XAMPP server. It's good looking theme I relay appreciate it. Nice work. I didn't find any major flaw/defect but need some usability improvement into theme like -

• Slider(rotating banner/slide) need controls to stop, forward, reverse slide.
• On theme setting page I try to edit and save external link/URL on banner(1) of slider into 'Banner Setting' but on front end it goes on 'page not found'. Please fix this.

Except above minor things theme is excellent!

itapplication
(Shiv)

Hi Lexicon,

I downloaded latest code/version of theme and tested it manually on my localhost XAMPP server. I saw you make appropriate changes as per my suggestions. Good work keep it up.

I found some other minor bugs into theme. You declare lot of regions but miss them into page.tpl.php file. For example 'Footer first column', 'Footer second column', 'Footer third column', 'Footer fourth column'. Where are these regions?
Please fix the regions bug.

For your reference I attache screenshot.

itapplication
(Shiv)

Automated Coder Sniffer found bugs at http://ventral.org/pareview/httpgitdrupalorgsandboxlexicon1960090git

Please fix the following bugs -

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

FILE: /var/www/drupal-7-pareview/pareview_temp/theme-settings.php
--------------------------------------------------------------------------------
FOUND 24 ERROR(S) AFFECTING 6 LINE(S)
--------------------------------------------------------------------------------
83 | ERROR | Spaces must be used to indent lines; tabs are not allowed
83 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
83 | ERROR | Array indentation error, expected 4 spaces but found 1
83 | ERROR | Expected 1 space after "=>"; 0 found
105 | ERROR | Spaces must be used to indent lines; tabs are not allowed
105 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
105 | ERROR | Array indentation error, expected 4 spaces but found 1
105 | ERROR | Expected 1 space after "=>"; 0 found
127 | ERROR | Spaces must be used to indent lines; tabs are not allowed
127 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
127 | ERROR | Array indentation error, expected 4 spaces but found 1
127 | ERROR | Expected 1 space after "=>"; 0 found
201 | ERROR | Spaces must be used to indent lines; tabs are not allowed
201 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
201 | ERROR | Array indentation error, expected 4 spaces but found 1
201 | ERROR | Expected 1 space after "=>"; 0 found
208 | ERROR | Spaces must be used to indent lines; tabs are not allowed
208 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
208 | ERROR | Array indentation error, expected 4 spaces but found 1
208 | ERROR | Expected 1 space after "=>"; 0 found
215 | ERROR | Spaces must be used to indent lines; tabs are not allowed
215 | ERROR | Line indented incorrectly; expected 2 spaces, found 1
215 | ERROR | Array indentation error, expected 4 spaces but found 1
215 | ERROR | Expected 1 space after "=>"; 0 found

Your project page could use some more information, it's very sparse - use text from the README?
The branch should be named 7.x-1.x instead of lexi-ect-7.x-1.x.
What is lexi.js? Perhaps you could use a more descriptive filename?

PAReview: 3rd party code

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

PAReview: Individual user account

It seems you are using a non-individual account.
All user accounts are for individuals. Accounts created for more than one user or those using anonymous mail services will be blocked when discovered (see Get a Drupal.org account).
Please change your account information and enter your realname and confirm that the lexicon account is a single user.

----
Top Shelf Modules - Enterprise modules from the community for the community.

Looks pretty sane to me, thanks for those updates.

Couple of small points:

• events_conferences_theme_settings_submit() is empty, you can probably remove that.
• In events_conferences_theme_field__taxonomy_term_reference() I think you can use theme('itemlist') to build your html list for you, instead of creating that manually.

----
Top Shelf Modules - Crafted, Curated, Contributed.

Thanks for those updates, looks good to me!

----
Top Shelf Modules - Crafted, Curated, Contributed.

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. Your readme is much better (but only few people take a look into them, so rather put that information on your project page)

Use commit messages to actually describe what you changed "Commited." is not helpful at all.

The contents of your t() calls are a bit messy..

t('Banner '),
t(' Image'),
t(' url '),
 t('Orgnization'),
t('Copyright '),

Remove the trailing/beginning spaces and correct the spelling

theme-settings.php

 245 /**
 246  * Functional conrol the form submission.
 247  */
 248 function events_conferences_theme_settings_submit() {
 249 
 250 }

Don't declare empty functions. Don't need it? remove it.

template.php

3 /**
   4  * @file
   5  * MyClass Class Doc Comment.
   6  * @link     http://www.lexiconsultants.com/
   7  */

What class doc comment? better commenting please

  27   /* Add conditional stylesheets for IE.*/
  28   drupal_add_css(
  29   path_to_theme() . '/css/ie.css',

Fix your indentation

You are using a lot of functions within your template...
eg. theme_get_setting(), drupal_get_path(), base_path()..
.. this is all stuff you can prepare in a preprocess function, put into a variable and then just print out to the user.
-> Separate logic from presentation.

Also you are not filtering user input properly.. you're directly printing out a lot of theme settings without proper sanitation.. you are not making sure that user entered paths are valid.. etc.
Although someone needs to have "Administer themes" permission to exploit these the drupal security team regards such weaknesses as actual security holes.

Therefore please read Writing secure code, Theming best practices and sub pages.

--------------
Moving to needs work.
Critical issue: Not filtering user input from theme settings properly before printing them out.

removed automated reviews.

You committed those fixes in the lexi-ect-7.x-1.x branch instead of the 7.x-1.x branch where they belong. Please commit that to 7.x-1.x and delete lexi-etc-7.x-1.x.

----
Top Shelf Modules - Crafted, Curated, Contributed.

Ventral.org link correctio

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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

Manual Review

typo on settings page

ORGNIZATION SETTING

should be ORGANIZATION SETTING

Manual review:

@file comment is not descriptive in theme-settings.php (line 10)

looks like should read:

/**
 * Implements hook_form_system_theme_settings_alter().
 *
 * Description here

But I can't find reference to this file elsewhere in theme, so maybe it is redundant?

Still coding standard issues reported by automated review: http://pareview.sh/pareview/httpgitdrupalorgsandboxlexicon1960090git

Removed space in URL to sandbox in issue summary and took it out of a code block to make it a link.

Those four minor white space issues are surely not application blockers, please do a real manual 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: User lexicon follows the guidelines for individual user accounts.

No multiple applications

No

No duplication

No

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code/content

(*) No: The directory images/banner contains three images thay may be copyrighted 3rd party material. I cannot find anything in README.txt about the ownership or licensing of these images, and whether they have been cleared for re-use. If they are own work, or 3rd party content available under GPL V2+. Reuse also requires that the license conditions is made clear and complied with. If these images are not cleared for reuse, they must be removed from the repo.

Note that 3rd party code or content 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.

Project page

(*) Please take a moment to make your project page follow tips for a great project page.

At the moment, it is very sparse. Is your theme HTML5 compatible? Is it responsive? Does it support SASS? Is it a starter theme, base theme or full theme? Is it configured through the command line or the GUI? Does it have any special features that makes it stand out? In particular, you should do more to guide developers looking for a theme about what your theme has to offer, and why they should choose it. See also review from kscheirer (#19) and from patrickd (#25) above about the project page.

README.txt

No: Please make your README.txt follow the guidelines for in-project documentation and the README.txt Template. As noted above, it also needs to list 3rd party material included with your theme. There are also multiple typos in README.txt. Use a spell-checker or do careful proofreading. You may also want to expand the section about getting a slider working by making the requirement/dependency on the slider module more explicit.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

No. See below.

Coding style & Drupal API usage

See below.

• (*) The copyright field is vulnerable to XSS exploits. If an attacker enters <script>alert('XSS');</script>, you will get a javascript popup alert. You need to sanitize user provided text before printing (it is printed unsanitized in page.tpl.php). Make sure you read Handle text in a secure fashion.
• Typo in theme-settings.php: 'copyright Treadmark' => 'copyright trademark'. But this is moot, since "copyright" and "trademark" are different things. This looks like a field to enter a copyright statement, and has nothing to with a trademark. Another typo is "Mani Slogan". Make sure you spell-check any user-facing text.
• Use of upper/lower case looks to be random (e.g. "Mani Slogan" vs "sub slogan"). Make up your mind whether to use title case, sentence case, or lower case for the names for fields, etc.
• I think it rather rigid to provide fields for just three specific social networks (Facebook, LinkedIn, and Twitter). You should either let the site maintainer select what networks or how many, or leave this out and instead suggest that one of the many flexible modules for having social links is used for this.

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

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

Changing status.

@klausi - #35 was never intended to be a review. I just started it, and had enough notes to know that it had to go back to "needs review", but was called away on a mission, and saved the text without thinking that I had already changed status.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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