[D7] S3 Zip Image Upload

Hello neha_patil,

My findings for your module as follows:

1) I see some strings where you should use the t() function.

Line No 175

form_set_error('folder_name', 'Unable to create directory. Please check public:// permissions');

2) Variable '$resize_filename' used at line no 212 is undefined.

3) Variable '$image' used at line no 212 is undefined.

Thanks

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgsandboxneha11212748397git

Fixed the git clone URL in the issue summary for non-maintainer users.

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.

Hi neha_patil,

Manual Review

1. Line 98 of s3_zip_image_upload.module has html tag
inside t() function. Referring here you need to only include human-readable text inside the t() function. You can concatenate the description like so
t("some description")."<br/>".t("some other description")

2. Line 144 of s3_zip_image_upload.module has html tag
inside t() function. Referring here you need to only include human-readable text inside the t() function. You can concatenate the description like so
t("some description")."<br/>".t("some other description")

Automatic Review

Major coding standards / best practice issues

An automated review of your project has found some issues with your code; As coding standards make sure projects are coded in a consistent style we ask you to please have a look at the report and try to fix them. However, please note that some of the issues found are possibly false positives and fixing all issues is not a requirement for getting through the application process.

You can find the results of the automated report at http://pareview.sh/pareview/httpsgitdrupalorgsandboxneha11212748397git

You can use Coder to automatically find and fix some of those issues for you.

PAReview: 3rd party code

*S3.php* appears to be 3rd party code. 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 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.

README.txt

Please take a moment to make your README.txt follow the guidelines for in-project documentation.

Automated review tool reported some issue http://pareview.sh/pareview/httpsgitdrupalorgsandboxneha11212748397git
Resolve those problems first.

Form and its callback should be written in .inc file not in .module file, only the hooks should be in module file, rest functions should be put in .inc file and this file must be included in module.

Automated review tool reported some issue
https://git.drupal.org/sandbox/neha1121/2748397.git
Kindly resolve this issues.

Hello neha_patil,

My findings for your module as follows:

File: s3_zip_image_upload.inc

Line 109 : Replace this $zip = new ZipArchive;
by $zip = new ZipArchive(); as parentheses is required if we call class constructors

File: s3_zip_image_upload.module

Line 97  and 100: Constants defined by a module must be prefixed with the module's name, so "AWSACCESSKEY" must be "S3_ZIP_IMAGE_UPLOAD_AWSACCESSKEY"

Line 105 and 106: Do not use mixed camle case, use lower case and _
i.e Replace $bDirectoryExists by $directory_exists

Line 131: drupal_set_message($message, 'error');
Instead of this use following
drupal_set_message(check_plain("$message"), 'error');

Thank you,

Also please check automated review and resolve them first.

See: http://pareview.sh/pareview/httpsgitdrupalorgsandboxneha11212748397git

Thank you,

It's working perfect for me, No code blocker.

Changing it's status for RTBC now.

@neha_patil thanks for your contribution.

@neha_patil, when you visit the project home page
https://www.drupal.org/sandbox/neha1121/2748397
one is not able to see your commits made on this project, this should be because of your git configurations on your machine.

Upgrading to critical since this hasn't received attention for over 4 weeks.

@neha_patil, yes one can see commits to this module on this https://www.drupal.org/node/2748397/commits page.

In #18, i was not mentioning this, please check below one
http://cgit.drupalcode.org/?q=neha-patil

Also, check this for configuring https://www.drupal.org/node/1022156

Removing review bonus tag, please add it after couple of reviews.

Marking it critical as there has been no activity over a month.

I will update your account so you can 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 stay involved!

Thank you, 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 go the dedicated reviewer(s) as well.