It is clean and light weight, while providing a basis for theme developers to develop.

Sanbox: http://drupal.org/sandbox/DaKo/1475150
Demo: drupal.koryto.eu

git clone --branch 6.x-1.x http://git.drupal.org/sandbox/DaKo/1475150.git green_worm

Comments

patrickd’s picture

Status: Needs review » Needs work

welcome,

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

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.
(At least this has to be done before switching back to needs review)

while waiting for an in-depht review of your module you can start out fixing some coding style issues detected by automated tools:
http://ventral.org/pareview/httpgitdrupalorgsandboxdako1475150git

You can also get a review bonus and we will come back to your application sooner.

regards

D4Ko’s picture

Status: Needs work » Needs review

Fix
new branch
git clone --branch work http://git.drupal.org/sandbox/DaKo/1475150.git green_worm

D4Ko’s picture

Issue summary: View changes

attachement warning

tonybuckingham’s picture

Status: Needs review » Needs work
StatusFileSize
new71.41 KB

Even though the branch is now called "work", it's still the master branch, as reported by:
http://ventral.org/pareview/httpgitdrupalorgsandboxdako1475150git
If you follow these instructions carefully:
http://drupal.org/node/1127732
. . . you should be good to go.

I installed the theme on a clean install of drupal 6.25 and the "content" div was pushed off to the right. I've attached a screen shot of the behavior. This css rule seems to be causing the problem:

Line 285, style.css:

/* sidebar */
.content {
  margin: 0;
  padding: 0;
  float: right;
  width: 240px;
}

I'm guessing you didn't mean to put that class in there (content probably shouldn't float right or have a width of only 240px). If I just delete those lines, things look better.

Line 8, block.tpl.php:

<h2 class="star"><?php print $block->subject ?></h2>

The css class "star" doesn't exist. Not sure if you intended to do something interesting with that and it's missing from the stylesheet or if it's there by accident.

Line 17, node.tpl.php:

    <?php if ($taxonomy): ?>
        <p class="tags"><?php print $terms; ?></p><?php endif;?>
    <?php endif; ?>

The "tags" class also doesn't exist. The $terms variable is an "unordered list" ( ul ), so wrapping it in a

doesn't actually do anything. I would just remove it. The css class for the term list is:

<ul class="links inline">
<li class="taxonomy_term_1 first"><a title="" rel="tag" href="/taxonomy/term/1">one</a></li>
<li class="taxonomy_term_3"><a title="" rel="tag" href="/taxonomy/term/3">three</a></li>
<li class="taxonomy_term_2 last"><a title="" rel="tag" href="/taxonomy/term/2">two</a></li>
</ul>

. . . so if you wanted to style the list, you could do something like:

.article .links {
   //something
}

Otherwise, pretty simple, straightforward theme . . .

tonybuckingham’s picture

Issue summary: View changes

Updated the description

D4Ko’s picture

Status: Needs work » Needs review

Fix
new branch
git clone --branch 6.x-1.1 http://git.drupal.org/sandbox/DaKo/1475150.git green_worm

patrickd’s picture

Now you named the branch like a tag ;-)
read again https://drupal.org/node/1015226

D4Ko’s picture

fix
git clone --branch 6.x-1.x http://git.drupal.org/sandbox/DaKo/1475150.git green_worm

D4Ko’s picture

Issue summary: View changes

New branch

nalan’s picture

StatusFileSize
new5.53 KB

Hi,
I just took a look at your theme, it looks simple and good. Here are some of my view points

Automatic review:

The following git branches do not match the release branch pattern, you should remove/rename them. See http://drupal.org/node/1015226

remotes/origin/6.x-1.1

Review of the 6.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:
1) It would be good if the readme file contains additional informations like:

  • its compatibility in different browsers
  • a few information about how to install the theme or a link to where it is explained in drupal.org
  • some more informations to describe the theme

2) The different template files (ie: *.tpl.php) could be place in separate folder called "templates" just like images and css folder.

D4Ko’s picture

Hi nalan,
Thanks for the advice. I made the appropriate corrections.
Automatic review: Fix
Manual review:
1) They have to be very detailed information?
2) It seems to me that the template.php file need not be in the templates directory.

nalan’s picture

Hi Dako,

Good to see that the coding styles has been fixed.

1) This link may help you to create the read me file guidelines for in-project documentation.
Try to give as much information as possible to help the user in using your theme. You can also check the readme file from other popular themes and analyse how they have structured and provided the information in the readme file.

2) Coming to template files, the "template.php" may stay in the root folder. I was mentioning the other template files (*.tpl.php), ie: block.tpl.php, block-content.tpl.php, node.tpl.php, page.tpl.php to be placed in the template folder.

D4Ko’s picture

Already it should be ok.

igoen’s picture

StatusFileSize
new54.18 KB
new58.95 KB

Just for suggestions.

Maybe you could add some block to the header.
Exactly for header in right corner.

See screenshoots.

You can add it with this step:

#page.tpl.php

<?php if ($right_header): ?>
    <div class="right_header">
      <?php print $right_header; ?>
    </div>
<?php endif; ?>

I add it below the

<div class="menu_nav">
    <?php print theme('links', $primary_links, array('class' => 'links primary-links')) ?>
</div>

so that right_header placed in line 31.

#green_worm.info

just add regions[right_header] = Right header

#style.css

.right_header {
  float: left;
  margin-left: 200px;
  width: 337px;
}
misc’s picture

Status: Needs review » Reviewed & tested by the community

Seems like RTBC for me.

klausi’s picture

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

klausi’s picture

Status: Reviewed & tested by the community » Fixed

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

Manual Review of the 6.x-1.x branch:

  1. Project page is too short, see http://drupal.org/node/997024
  2. "$vars['authored'] = t('Submitted by') . ' ' . $vars['name'];": do not concatenate variables into translatable strings, use placeholders with t() instead.
  3. green_worm_preprocess_node(): $vars is passed by reference here, so it does not make sense to return them?

But that are not hard application blockers, so ...

Thanks for your contribution, DaKo!

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:

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.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

correct banch name