Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Lucid is a new approach when it comes to drupal themes. With Lucid we wanted to try to move away from using base themes, and instead build a good start out point for whatever theme you are building. We believe that themes don't have the necessary upgrade path for newer version that modules have. After downloading Lucid, you are free to keep the parts you like, and change the parts you don't like.
Besides a great reset.css file, Lucid provides a nice looking default style for all elements, i.e tables, forms and tabs. Lucid also cleans up a lot of markup and css classes provides by both core and contrib modules.
Lucid also has another niche. It is designed to go hand in hand with panels and panels everywhere. Besides providing it's own panel layouts, it's arranged to work great with the NodeStream distribution.
One site, one theme, one love!
Project sandbox: http://drupal.org/sandbox/Haugen/1302252
Clone repository: git clone --branch 7.x-1.x Haugen@git.drupal.org:sandbox/Haugen/1302252.git lucid
Drupal version: 7.x
Comments
Comment #1
doitDave CreditAttribution: doitDave commentedOK, here we go!
Automated review (Please keep in mind that this is primarily a high level check that does not replace but, after all, eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.)
It 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.
Review of the master branch:
This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.
Comment #2
whitefluffy CreditAttribution: whitefluffy commentedThese issues have been taken care of. I have also added a new branch for the project.
Clone repository: git clone --branch 7.x-1.x Haugen@git.drupal.org:sandbox/Haugen/1302252.git lucid
Comment #3
whitefluffy CreditAttribution: whitefluffy commentedChanging status!
Comment #4
raynimmo CreditAttribution: raynimmo commentedAutomated review
Please keep in mind that this is primarily a high level check that does not replace but eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.
Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):
These are fairly minor points really, they seem to possibly be due to the windows style line breaks that are within your code. These should be changed to unix style line breaks. Make sure your text editor is set up correctly to format your files in this manner upon saving.
*note The error generated above for \lucid\js\lucid\html5.js is because the full code is commented out. If this file is not being used then you should possibly remove it.
I noticed that html5.js and respond.js are both licenced under the dual MIT/GPLv2 licence, this should be ok to include but you may want to check first as 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 that 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.
CSS Coding Style
I realise it is not a strict prerequisite for many reviewers but you should maybe make an attempt to adhere to the CSS coding standards.
Also, you include the browser specific styles -webkit and -moz but have omitted to include any for other browsers. Possibly have a read of http://reference.sitepoint.com/css/vendorspecific to make your theme a bit more cross browser compatible.
Make sure to set your status back to 'needs review' once you have addressed the issues highlighted above.
Good luck with the rest of your review.
Comment #5
whitefluffy CreditAttribution: whitefluffy commentedThank you for your review.
I was unable to reproduce the issues of the file endings. I have used both coder and code sniffer and none of them are reporting any errors as of now, as far as I can tell. The following issues has been taken care of though:
Other than that we have moved the html5.js and the respond.js files out of the project. There has also been minor fixes, i.e making css align with the Drupal coding standard.
Moving back to needs review.
Comment #6
aliyayasir CreditAttribution: aliyayasir commentedThere are still files other than README.txt in the master branch, make sure to remove them. See also step 5 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:
Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):
This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.
Source: http://ventral.org/pareview - PAReview.sh online service
Comment #7
whitefluffy CreditAttribution: whitefluffy commentedThe master branch has been cleared and these issues should be OK now. Setting back to needs review.
Comment #8
klausilucid_html_head_alter(): this is a hook implementation and should be documented as such, see http://drupal.org/node/1354#hookimpl . Please also check your other functions.
Otherwise looks good to me.
Comment #9
zzolo CreditAttribution: zzolo commentedHey @heyintern, you have been given full Git access. Happy hunting
Thanks to all the reviewers!
Comment #10.0
(not verified) CreditAttribution: commentedchanged branch in link to repository