[D7] Imgfly

Hey thamba. I'm confused - what does this module do that isn't already accomplished with css "img{max-width:100%}"?

I activated this module, added an image style with the "_imgfly" suffix, and made sure it was set to scale and crop - I also have images set to max-width:100%. I set the size to 500x500, but didn't see that the effect was any different when I resized, than with plain css. Apologies if I'm doing something incorrectly.

Maybe modify your script to add states to Drupal.settings, and serve different image_cache styles based on screensize?

Gotcha - now I see it.

I do a lot of themeing for responsive sites, and we generally fire responsive js scripts not only on window load / page ready, but also on resize. It would be pretty easy to add your image function to jQuery's resize().....but I hear what you are saying.

Hello there!

The automatic review says, that your module is clean. :-)

Manual review of 7.x-1.x branch:

• imgfly.admin.inc, LINE 31, maybe add a text, that you expect the dimension in the format 100x100.
• imgfly.module, LINE 55, I think it would be better to use boolean values here.
• imgfly.info, I think modules working with images would normally go into the package = Media

I know, that you are already handling it to some point by restricting the image size to a certain dimension, but as far as I understand it your module could still easily be exploited to attack a server. A possible attacker could just go through all permutations of image dimensions and let your script render these images. Not sure, if it's worth fixing and if there is an easy way to avoid this, though. :-)

Hello @thamba,
Your project is interesting and andmore your code is clean.
In addition to optimization remarks from @Stefan Lehmann I add this :
LINE 119 : Add a test before your stock variable
$itok = isset($_GET['itok']) ? trim($_GET['itok']) : '';

manual review:

1. please also mention the differences to the existing projects on your project page.
2. imgfly_generate(): @return resource: what resource is returned? The function does not return anything? See http://drupal.org/node/1354#return
3. imgfly_admin(): all variables defined by your module need to be removed in hook_uninstall().

But otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to sreynen as he might have time to take a final look at this.

Thanks for your contribution, thamba !

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 continuing to review 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 reviewers as well.