Project link: http://drupal.org/sandbox/EricoVinicius/1111334
Vintage is an D6 starter theme with an attitude that is build on top of the HTML5 boilerplate and the Basic theme.
It also include some extra features like collapsible blocks with cookies, setting to fixed or fluid width thought the settings page, and dynamic font replacement with google web-fonts.
Screenshot attached.
| Comment | File | Size | Author |
|---|---|---|---|
| #26 | spacing.patch | 37.38 KB | andrewyager |
| Screen shot 2011-03-31 at 9.57.02 AM.png | 588.08 KB | EricoVinicius |
Comments
Comment #1
Arnold Leung commentedComment #2
laura s commentedOverall this looks pretty good. In IRC @Heine and I noted a potential issue with passing args. I posted in the project issue queue.
Comment #3
EricoVinicius commentedArgs issue fixed.
Comment #4
EricoVinicius commentedArgs issue fixed.
Comment #5
Arnold Leung commentedStill needs reviewed before marking as RTBC..
Comment #6
Arnold Leung commentedAny news with this?
Comment #7
meba commentedI seriously dislike this in template.php:
Apart from the fact that ereg was deprecated, I don't want to think about the fact that $term->name is insecure. It is mitigated by the ereg but still, we don't want to open a can of worms
Once this is fixed, I give it RTBC
Comment #8
EricoVinicius commentedUpdated to:
Comment #9
meba commentedRtbc imho
Comment #10
dave reidLooks like there is a possible XSS vulnerability - the footer_copyright() does not escape the site_name variable:
Comment #11
EricoVinicius commentedUpdated line 477 to:
return t('Copyright ') . $year . ' ' . check_plain(variable_get('site_name', 'drupal')) . '.';
Comment #12
EricoVinicius commentedThanks for all the feedback guys, its been a very interesting experience. Good to be involved with the community.
Any more info on this? Are we ready to go?
Thx!
Erico Nascimento
Comment #13
EricoVinicius commentedNo more feedback?
Comment #14
Arnold Leung commentedComment #15
sreynen commentedEverything is normal priority in this queue.
Comment #16
sreynen commentedTagging for easier finding.
Comment #17
joachim commentedThat's not a theme's job.
That hits the database, and if you have a $node in the vars, then it already contains the taxonomy terms!
Also, why the preg if you have vintage_id_safe() to hand?
A lot of work round here just to clean up classes on node add / edit / delete. Is ability to theme those pages so important?
This bunch of checks needs to be inside a check that node exists.
- Bad indentation in vintage_preprocess_block() and vintage_links()
- Review how you lay out docblock comments -- only one space after the initial * and no blank line before the function declaration.
- vintage_breadcrumb missing docblock
Overall, I'm not sure this qualifies as a *starter* theme... there's a lot in there that I'd find I don't need or would outright have to revert.
Comment #18
EricoVinicius commentedRemoved:
foreach (taxonomy_node_get_terms($vars['node']) as $term) {updated with
foreach (($vars['node']->taxonomy) as $term) {Replaced: preg_replace() with vindage_id_safe();
Indentation fixed.
docblocks fixed
Comment #19
sarah_p commentedComment #20
sarah_p commentedComment #21
EricoVinicius commentedAny more feedback?
Comment #22
ccardea commentedIndentation went from bad to worse. Each indent should be 2 spaces. I don't think we need to hold up this application for indentation issues. Several people have looked at this and all of the issues have been addressed. It would be nice if reviewers would follow up.
Comment #23
tim.plunkettIf they think that the indentation was fixed, it becomes a bigger problem than if it was just wrong.
Comment #24
klausiNo activity in several months. Reopen and set the status to "needs review" if you are still pursuing this application.
Comment #25
Arnold Leung commentedComment #26
andrewyager commentedPatch applied to fix indentation, the issue is most likely that their editor replaced double spaces with \t characters.
Comment #27
andrewyager commentedThe above patch does not fix all of the code formatting problems; check for tabs that should be spaces. Also there was a number of relatively minor formatting issues in your template file. The coder module (drupal.org/project/coder) will help you find the specific formatting issues.
It also 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.
Good work though :) Keep at it!
Comment #28
misc commented@EricoVinicius has been contacted to ask if the application is abandoned.
http://drupal.org/node/894256
Comment #29
gregglesFor #10.
Comment #30
klausiClosing due to lack of activity. Feel free to reopen if you are still working on this application.