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.

CommentFileSizeAuthor
#10 screenshot.png94.36 KBAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Title: Project Access » Rasp
Category: support » task
Status: Active » Needs review
PA robot’s picture

Status: Needs review » Needs work

There 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.

Anonymous’s picture

Status: Needs work » Needs review
aramboyajyan’s picture

I 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:

  1. 1. The “Coder” module complains about the “@file blocks”. They are missing in the following theme files:
    • example.js
    • page.preprocess.inc
    • page.process.inc

    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.

  2. The following README.txt files are empty (content: “Fill me”):
    • css/README.txt
    • images/README.txt
    • js/README.txt
    • templates/README.txt

    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.

  3. I'd clean the logs from the css/no-query.css.
  4. Lastly, one personal note: the theme root might be more organized if you would group subfolders. For example: css, fonts, images, js, libraries to assets or whatever else that makes sense to you. Right now there are 10 folders + 8 files, making the tree fairly long.

    This is not a direct issue, rather organizational point.

Let me know if you have any questions regarding the feedback.

Anonymous’s picture

Status: Needs review » Needs work

Thank 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 from styles.scss to style.scss Sass produced some compilation error. This file was not included in rasp.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.

Anonymous’s picture

Status: Needs work » Needs review
aramboyajyan’s picture

You are welcome, I'm glad the feedback was useful.

aramboyajyan’s picture

One 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.:

  • Twitter Bootstrap: //cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/3.0.0/js/bootstrap.min.js
  • Modernizr: //cdnjs.cloudflare.com/ajax/libs/modernizr/2.6.2/modernizr.min.js or as Drupal module: https://drupal.org/project/modernizr
  • HTML5 Shiv: //html5shiv.googlecode.com/svn/trunk/html5.js
  • Pie: //cdnjs.cloudflare.com/ajax/libs/pie/1.0beta5/PIE.js
  • RespondJS: //cdnjs.cloudflare.com/ajax/libs/respond.js/1.3.0/respond.js or as Drupal module: https://drupal.org/project/respondjs
  • Selectivizr: //cdnjs.cloudflare.com/ajax/libs/selectivizr/1.0.2/selectivizr-min.js or as Drupal module: https://drupal.org/project/selectivizr

The 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.

Anonymous’s picture

Hi 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.

Anonymous’s picture

Title: Rasp » [D7]Rasp
FileSize
94.36 KB
kscheirer’s picture

Title: [D7]Rasp » [D7] Rasp

Just cleaning up issue title.

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community
klausi’s picture

Status: Reviewed & tested by the community » Needs review

Please don't RTBC your on issues, see https://drupal.org/node/532400

dubcanada’s picture

Status: Needs review » Needs work

In 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.

Anonymous’s picture

Priority: Normal » Critical
Status: Needs work » Needs review

R U F K M???

what JavaScript cleaning? be specific, which multiple files not used? what's that with commented line?

kscheirer’s picture

Priority: Critical » Normal

Critical 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".

Anonymous’s picture

so you mean the cycle starts again due to some psycopaths' trespassing to show his ponytail?

Anonymous’s picture

Priority: Normal » Critical

It 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.

kscheirer’s picture

It 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.

oresh’s picture

Status: Needs review » Needs work

Please 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.

oresh’s picture

your 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?

PA robot’s picture

Issue summary: View changes
Status: Needs work » Closed (won't fix)

Closing 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.

Anonymous’s picture

Assigned: Unassigned »
Priority: Critical » Minor
Status: Closed (won't fix) » Postponed

due to lack of time, I am postponing this for now. Will check back later

PA robot’s picture

Status: Postponed » Closed (won't fix)

Closing 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.