Closed (fixed)
Project:
Drupal.org security advisory coverage applications
Component:
theme
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
8 Nov 2012 at 14:54 UTC
Updated:
4 Jan 2014 at 02:42 UTC
Jump to comment: Most recent


Comments
Comment #1
jsacksick commentedPlease fix the coding style issues detected by automated tool : http://ventral.org/pareview/httpgitdrupalorgsandboxtedavis1833576git
Comment #2
Anonymous (not verified) commentedCoding style issues should now be resolved.
Comment #2.0
Anonymous (not verified) commentedChanged image size
Comment #2.1
Anonymous (not verified) commentedAdded in first review
Comment #3
anton-staroverov commentedHi tedavis,
1) template.php: I think it's not good to call drupal_add_css() in the global context. Place it inside elementary_preprocess_page()
2) block.tpl.php: in templates you should to add semicolon (;) after all ending clauses and also leave one space after them
3) it's very minor but you should use
<code>tag for your git instructions on project pageComment #4
anton-staroverov commentedComment #5
Anonymous (not verified) commented@tonystar thanks for the review,
1. Changed.
2. Added the additional semicolon but it actually looks like the core block.tpl.php is missing this too, a bug with core perhaps?
3. Not sure what you mean with this, my sandbox page only has the default git instructions provided by d.o & I did use the code tag for this issue.
Comment #5.0
Anonymous (not verified) commentedAnother review added
Comment #6
Anonymous (not verified) commentedAdded in project reviews
Comment #7
stixes commentedAutomated:
Automated review with PAReview comes up clean.
Manual:
Looks pretty good. I would like an easy option for subtheming. Being minimalist, it seems obvious to include a starterkit subthemes (also, it a rather simple thing to do), however it's not really a requirement for review.
I would need to see the timestamps locallized. Either by adding a custom format or using the standards.
Documentation:
Proper formatted, and decent explainations. I do think the term "super simple theme" used on the project page should be replaced by "minimalistic" as used in the README, since "simple" can be alot of things, not just layout related.
Comment #8
stred commentedthese are only remarks, nothing critical here...
Comment #9
Anonymous (not verified) commented@stred Thanks for the comments, much appreciated.
I've now removed the additional screenshot file.
hook_preprocess_textarea was an incorrect comment on my part - should have been theme_textarea. All this does is remove the grippie from the text area, it should still be resizable by default in modern browsers so the grippie isn't really needed anymore.
Just updated the padding for the tabs which should now fall into place & the permission should be correct now.
In terms of the date, I do see what you mean in terms of flexibility but when I had it set to the default it just didn't seem to look right, there was too much going on. I'll definitely have a think though, if more people would like the flexibility I'll probably put it back.
Let me know if you find anything else!
Comment #10
stred commentedyou could use the format_date with 'short' instead of 'custom' so people can easily modify the display as they like... but anyway I change the status to reviewed.
Comment #11
klausiThanks for your contribution, tedavis!
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 #12
Anonymous (not verified) commentedAwesome, thank you!
Comment #13.0
(not verified) commentedAdded 3rd project review