[D7] CSS Crush

Hi,

Remove LICENSE.txt, it will be added automatically.

An automated review of your project has found some issues with your code. See http://ventral.org/pareview/httpgitdrupalorgsandboxpeteboere1740930git.

http://ventral.org/pareview/httpgitdrupalorgsandboxpeteboere1740930git is still showing issues. I'd suggest running it through the Coder module (http://drupal.org/project/coder) to ensure that it complies with Drupal's coding standards.

One small issue, when I try to enable this module I get a fatal error with the following message. Call to undefined function csscrush_bootstrap().

Perhaps a more experienced Drupaller could correct me, but I figure a call to something like the following inside csscrush.install would correct the issue?

module_load_include('module', 'csscrush', 'csscrush');

Or perhaps the csscrush_bootstrap() function could be factored out into a separate .inc file?

Otherwise, I've read over the module code, and it looks pretty solid in general.

You may consider to add :-

git clone --recursive --branch 7.x-1.x http://git.drupal.org/sandbox/peteboere/1740930.git css_crush

command in your issue summery, so that reviewers can directly copy paste :)

--

While trying to install I got this error - http://www.diigo.com/item/t/4392082_138848861_8187317

This is probably that I have not added the csscrush inside (library) folder.

You must mention in your README.txt and also in the summery of your project that.

"Please download csscrush library from -givenlocation- and putit inside the libraries folder. with the name "csscrush"

Another thing, I saw that you are using
libraries_get_path('CssCrush'), $search_paths[] = "sites/all/libraries/CssCrush/$target";
in your code, people tend to use lowercase letter to name the folders, hence if I am in a linux server and if I named my library folder as 'csscrush' it may give error.

So be specific about the library foldername as well, I would suggest you use "csscrush" everywhere and instruct users to download it and have it ready before using your module.

Thanks

Nice, your README.txt file has much more clear instructions now, I will suggest once this project goes live, put the same info in project description, as many beginners and you know most developers are lazy and forget to bother about README.txt and will keep blaming about the documentation, so if you have it in your project description, it is all set.

Above is my personal suggestion though.

PS : I am always eager to help, collaborate, contribute, feel free to contact me if I can help you anywhere, lets share our knowledge and keep helping the community :)

Thanks

Hi,

It looks like a very nice project that probably will help many drupal users.

Please notice that there are minor issues in your automatic report: http://ventral.org/pareview/httpgitdrupalorgsandboxpeteboere1740930git

Please notice several issues that you may consider to take care of:
1. csscrush_requirements: from security point of view it's better to perform if () return rather than canonical if. In this case it can help you avoid redundant code, since you check $library_available once, and take care of this case.
2. csscrush_add_css: same. You may also consider refactor this function
3. _csscrush_include_path: many canonical if statements. can be easily reduced to simple if () return;
4. _csscrush_include_path: line #201 seems redundant and should be removed. lines 204+205 can me merged.

Best Regards,
Moshe

Changed to needs work.
Consider doing http://drupal.org/node/1410826 to accelerate your acceptance process

Manual Review

Hi pete-b

i try your Module. after downloading csscrush from git and put it to my libraries directory i get this error

Fatal error: require_once() [function.require]: Failed opening required './sites/all/libraries/csscrush/lib/CssCrush/Io.php' (include_path='.:/usr/local/lib/php') in./sites/all/libraries/csscrush/CssCrush.php on line 18

i try also like you in the readme.txt file says to put it in the module folder
this brings me the site again but with this errors

    Warning: require_once(./sites/all/libraries/csscrush/lib/CssCrush/Io.php) [function.require-once]: failed to open stream: No such file or directory in csscrush_autoload() (Zeile 18 von ./sites/all/libraries/csscrush/CssCrush.php).
    Warning: require_once(./sites/all/libraries/csscrush/lib/CssCrush/CssCrush/Version.php) [function.require-once]: failed to open stream: No such file or directory in csscrush_autoload() (Zeile 18 von ./sites/all/libraries/csscrush/CssCrush.php).
    Warning: require_once(./sites/all/libraries/csscrush/lib/CssCrush/Io.php) [function.require-once]: failed to open stream: No such file or directory in csscrush_autoload() (Zeile 18 von ./sites/all/libraries/csscrush/CssCrush.php).
    Warning: require_once(./sites/all/libraries/csscrush/lib/CssCrush/Io.php) [function.require-once]: failed to open stream: No such file or directory in csscrush_autoload() (Zeile 18 von ./sites/all/libraries/csscrush/CssCrush.php).
    Warning: require_once(./sites/all/libraries/csscrush/lib/CssCrush/Io.php) [function.require-once]: failed to open stream: No such file or directory in csscrush_autoload() (Zeile 18 von ./sites/all/libraries/csscrush/CssCrush.php).
    CSS Crush library files not found. Download the latest stable release from Github: https://github.com/peteboere/css-crush/zipball/master (Momentan wird CSS Crush version n. v. verwendet)

when i refresh the page only this error comes up

Fehlermeldung
CSS Crush library files not found. Download the latest stable release from Github: https://github.com/peteboere/css-crush/zipball/master (Momentan wird CSS Crush version n. v. verwendet)

the module i can´t enable!!!

Please use and make it working only with the libraries module...i think this is better for the folks and with path setup for you.

Inside of ccscrush.module you use this

// Inside the module as last resort.
    $search_paths[] = drupal_get_path('module', 'csscrush') . "/lib/csscrush/$target";

i mean thats wrong and need to call /lib/CssCrush/

hope this helps...using this on a live site

Good Luck

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Hi pete boere

i try it again. installation works fine now!i use omega theme. i setup like you say in my template.php file these lines.

/*
Adding a CSS Crush processed file to your theme (ideally in template.php).
*/
csscrush_add_css('theme://your-theme/styles.css');
i add also this line csscrush_add_css('theme://spar/css/global.css');

and i try these lines

function spar_preprocess_html(&$variables) {
// Add conditional stylesheets for IE
$theme_path = drupal_get_path('theme', 'spar');
csscrush_add_css($theme_path . '/css/global.css');

after all i become a green msg under my admin...i mean must be red?

Statusmeldung Debug: CssCrush::loadAssets: Plugin file could not be parsed. in CssCrush::loadAssets() (Zeile 212 von ./sites/all/libraries/csscrush/lib/CssCrush/Core.php).

maybe i make something wrong?

Looks like a solid module.

csscrush-adapter.inc

• Perhaps swap out md5 usage for drupal_hash_base64(). See: http://drupal.org/node/845876
• cache_clear_all() should mention the cache bin. See: http://api.drupal.org/api/drupal/includes!cache.inc/function/cache_clear...

Project review

automated review

No errors found.

Manual review

• As a best practice take all non-hook functions in your module file and place them in csscrush.functions.inc or similarly named file. Seeing a function called csscrush_bootstrap in the module file lead me to think this is a hook function. Similarly the csscrush_add_css function made me think you were injecting your own special css files into drupal without developers needing to do much. I assume as a developer I have to call csscrush_add_css with my special CSS file directly to make use of your module. Ahh, thats covered in the README.
  
  I still recommend the refactoring of drupal hooks from your "csscrush" module funtions into seperate files.
• The CssCrushDrupalIo class file should be included in the files[] array of your module info file to be registered by Drupal.
• From the code, I cant tell where you're trying to save the generated CSS files. Ideally, they should be within a subdirectory of a sites files sub directory. You're not guarenteed to have write access to a module folder or theme folder.
• consider implementing hook_help
• consider adding the package package = Development to your info file to classify your module as a developer related module in the module list of a drupal site.

Thanks for the useful module for debugging.

oops forgot to change status in #24.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

I'm a robot and this is an automated message from Project Applications Scraper.

Major is only for issues that haven't been reviewed in 2 weeks, see https://drupal.org/node/539608#application-priorities.

• Do you want to use file_directory_temp() instead of the public files directory?
• Do you need to load csscrush.api on every page request?
• In csscrush_add_css(), it looks like the default file type is file. Why not use that in the function arguments like $options = array('type' => 'file');
• You can use drupal_static() for your static vars, but I'm not sure there's any need to.
• You should remove csscrush_source_map in hook_uninstall()
• You have getCacheData and setCacheData defined in your adapter, but not used anywhere? I guess you're not caching anything, using statics instead.

Looks like a nice module! None of those are blocking issues.

----
Top Shelf Modules - Crafted, Curated, Contributed.

It's been a month without any problems reported, so I'm promoting this myself as per https://drupal.org/node/1125818.

Thanks for your contribution, pete-b!

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:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

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.

----
Top Shelf Modules - Crafted, Curated, Contributed.