Closed (fixed)
Project:
Drupal.org security advisory coverage applications
Component:
theme
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
9 Mar 2012 at 14:20 UTC
Updated:
14 Aug 2012 at 20:51 UTC
Jump to comment: Most recent file
Comments
Comment #1
patrickd commentedwelcome,
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
Comment #2
D4Ko commentedFix
new branch
git clone --branch work http://git.drupal.org/sandbox/DaKo/1475150.git green_worm
Comment #2.0
D4Ko commentedattachement warning
Comment #3
tonybuckingham commentedEven 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:
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:
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:
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:
. . . so if you wanted to style the list, you could do something like:
Otherwise, pretty simple, straightforward theme . . .
Comment #3.0
tonybuckingham commentedUpdated the description
Comment #4
D4Ko commentedFix
new branch
git clone --branch 6.x-1.1 http://git.drupal.org/sandbox/DaKo/1475150.git green_worm
Comment #5
patrickd commentedNow you named the branch like a tag ;-)
read again https://drupal.org/node/1015226
Comment #6
D4Ko commentedfix
git clone --branch 6.x-1.x http://git.drupal.org/sandbox/DaKo/1475150.git green_worm
Comment #6.0
D4Ko commentedNew branch
Comment #7
nalan commentedHi,
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:
2) The different template files (ie: *.tpl.php) could be place in separate folder called "templates" just like images and css folder.
Comment #8
D4Ko commentedHi 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.
Comment #9
nalan commentedHi 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.
Comment #10
D4Ko commentedAlready it should be ok.
Comment #11
igoen commentedJust 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
I add it below the
so that right_header placed in line 31.
#green_worm.info
just add
regions[right_header] = Right header#style.css
Comment #12
misc commentedSeems like RTBC for me.
Comment #13
klausiWe 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 :-)
Comment #14
klausiSorry 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:
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.
Comment #15.0
(not verified) commentedcorrect banch name