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.
I have made a starter theme based on Omega 4.x and Bootstrap 3.0.0. I am looking forward to post this as full Drupal project. The link for the Sandbox project is: https://drupal.org/sandbox/rajibmp/2094841.
If you have any questions or Suggestions then please let me know.
Comment | File | Size | Author |
---|---|---|---|
#10 | screenshot.png | 94.36 KB | Anonymous (not verified) |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #2
PA robot CreditAttribution: PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxrajibmp2094841git
We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)
Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #3
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #4
aramboyajyan CreditAttribution: aramboyajyan commentedI checked the theme and it seems to be pretty much wrapped up.
Apart from what the robot reported, there are just a few minor code-related things that you should fix so the automated services don't report any issues:
It also reported that they are missing in the JS libraries, but I'd ignore that as it's better to leave those files unmodified.
These two folders are very little likely to be filled by the theme authors, so I believe it would be better to include some short description instead.
If you just want to leave those folders empty, you can remove README.txt and place a .gitkeep file instead.
css/no-query.css
.This is not a direct issue, rather organizational point.
Let me know if you have any questions regarding the feedback.
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedThank you for the feedback, it is really helpful.
Yes, I could have used .gitkeep instead of empty README.txt, and will do that.
Nice catch about
css/no-query.css
file, actually it should be another compiled CSS file, and when I changed the name fromstyles.scss
tostyle.scss
Sass produced some compilation error. This file was not included inrasp.info
so, the theme layout didn't produce any error. I will fix that.About the folder organization, Its good to organize like that as you said, but personally I prefer this way, its not confusing for people to find around where is what. After all its just 10 folders and highly unlikely to increase beyond that.
Comment #6
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #7
aramboyajyan CreditAttribution: aramboyajyan commentedYou are welcome, I'm glad the feedback was useful.
Comment #8
aramboyajyan CreditAttribution: aramboyajyan commentedOne more thing to change are the included libraries: you might want to require the user to add them through Libraries API module or you can simply pull them from CDN.
Most of the libraries you included in the theme are available at some CDN or as a standalone Drupal module that uses Libraries API, e.g.:
//cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/3.0.0/js/bootstrap.min.js
//cdnjs.cloudflare.com/ajax/libs/modernizr/2.6.2/modernizr.min.js
or as Drupal module: https://drupal.org/project/modernizr//html5shiv.googlecode.com/svn/trunk/html5.js
//cdnjs.cloudflare.com/ajax/libs/pie/1.0beta5/PIE.js
//cdnjs.cloudflare.com/ajax/libs/respond.js/1.3.0/respond.js
or as Drupal module: https://drupal.org/project/respondjs//cdnjs.cloudflare.com/ajax/libs/selectivizr/1.0.2/selectivizr-min.js
or as Drupal module: https://drupal.org/project/selectivizrThe only one I couldn't find at some CDN or Drupal module is matchmedia polyfill script.
Even though these are all front-end scripts and there is little chance that some module will require them (causing duplicates and possibly issues/conflicts), try doing this the Drupal way as much as possible.
I'd include all libraries available in separate modules that way instead of placing the file in the theme.
Hope this makes sense.
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedHi again,
thank you for the review.
Those js libraries are not essential to the theme, I included it because I was using it. But now I have made separate git branch for my project, I removed all those unnecessary libraries.
You are right about uniformity in application and avoiding conflicts by using libraries or modules which are already available.
I am still learning the process, and thanks for the help and comments.
Comment #10
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #11
kscheirerJust cleaning up issue title.
Comment #12
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #13
klausiPlease don't RTBC your on issues, see https://drupal.org/node/532400
Comment #14
dubcanada CreditAttribution: dubcanada commentedIn your .info file you don't need
engine = phptemplate
And you shouldn't have commented out lines by default. Just remove them.
;scripts[] = libraries/bootstrap/bootstrap.min.js
;scripts[] = js/script.js
I would also suggest cleaning up the theme a little, right now there are multiple files that are not used. Most of the javascript can be removed.
Comment #15
Anonymous (not verified) CreditAttribution: Anonymous commentedR U F K M???
what JavaScript cleaning? be specific, which multiple files not used? what's that with commented line?
Comment #16
kscheirerCritical is only for issues that haven't been reviewed in 4 weeks, see https://drupal.org/node/539608#application-priorities. I agree that those were not blocking issues, and this should have stayed in "needs review".
Comment #17
Anonymous (not verified) CreditAttribution: Anonymous commentedso you mean the cycle starts again due to some psycopaths' trespassing to show his ponytail?
Comment #18
Anonymous (not verified) CreditAttribution: Anonymous commentedIt has already been 4 weeks since last meaningful review has taken place and I have addressed the suggestion. Since then somebody came to clean the title and somebody came to collect some review point without knowing much things.
So, I am changing this back to critical.
Comment #19
kscheirerIt can take a while, everyone in this queue is a volunteer, please be patient. 1-3 months is very common before an application is finally approved. The best thing you can do is get a Review Bonus by reviewing other applications. That will get you to the top of the list of projects to get reviewed (and hopefully approved). Only manual reviews count, just using http://pareview.sh is not enough.
After 2 weeks of no review, you can bump the priority up again.
Comment #20
oresh CreditAttribution: oresh commentedPlease add git clone link and link to your commits (https://drupal.org/node/1011698 5.4)
Your branch should be 7.x-1.x (https://drupal.org/node/1587704 2.2)
Please don't have commented scripts[] in your info file
Bootstrap, html5shiv, respond and other libraries should either be included as CDN link or vie libraries module. (https://drupal.org/node/1587704 4.2)
Full review coming next.
Comment #21
oresh CreditAttribution: oresh commentedyour no-query.css and style.css are equal. Are you sure you have to have them both?
Both of these files could have no comments - they are for debugging use mostly and not for production theme.
in /preprocess, /process and /theme folder change the readme.md to readme.txt
I don't quite understand what should be reviewed here... You have lots of libraries and bootstrap sass files, and empty folders and files your theme variables.
Except the libraries, doesn't bootstrap/omega theme cover all of that?
Comment #22
PA robot CreditAttribution: PA robot commentedClosing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #23
Anonymous (not verified) CreditAttribution: Anonymous commenteddue to lack of time, I am postponing this for now. Will check back later
Comment #24
PA robot CreditAttribution: PA robot commentedClosing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).
I'm a robot and this is an automated message from Project Applications Scraper.