[D7] IXI: XML Importer

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

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,

I had reviewed your module, here is my feedback :
a. Remove files[] array from ixi.info
b. Remove package information from ixi.info
c. Remove files[] array from ixi_features.info
d. Line no 65 ixi_reference/ixi_reference.module not sure why you are using $node->language instead of LANGUAGE_NONE constant.
e. Two READ files are there, remove one of them.
f. Remove files[] array from ixi_upload/ixi_upload.info
g. Remove package information from ixi_upload/ixi_upload.info
h. Remove package information from ixi_features.info
i. Use db_select instead of db_query and db_query_range so it compatibles to other other db engines.

Manual review:
1. Your folder name in git is 2328781, please change that to ixi
2. The includes files should be declared in .info file like:

files[] = ixi.admin.inc

3. You can use drupal_goto function instead of

$form_state['redirect'] = 'admin/reports/status';

4. You used old fashioned sql select queries, which i am not against at all, but someone sure will ask you to use db_select function, see https://api.drupal.org/api/drupal/includes%21database%21database.inc/fun...

Auto review

Please fix the issues in pareview.sh http://pareview.sh/pareview/httpgitdrupalorgsandboxsupermario2328781git

@bekirdag: ouch, almost all of that advice is wrong.

1) The folder name is determined by the git clone command and cannot be changed in the repo itself.
2) Only files containing classes should be listed in the info file.
3) do NOT use drupal_goto() in form submit handlers, $form_state['redirect'] is the correct way.
4) do NOT use db_select() for simple static queries, see https://www.drupal.org/node/310075

Leaving at "needs work" for the coding standard fixes reported by pareview.sh.

Updated issue summary.

Removing review bonus tag as @bekirdag added it, @super_mario you can add it, if you have done 3 reviews of other projects.

@bekirdag, I noticed your other reviews also, you are adding PAReview: review bonus tag everywhere, please don't do that. Only author of project will add this after doing 3 reviews of other projects.

@er.pushpinderrana sorry for the confusion, will do in the future.

@klausi thanks for clarification.

updated issue summary.

@super_mario,
Thanks for your contribution.
Change the issue status after fixing the suggestions as mentioned by @klausi in #6.Also fix the automated Review by pareview http://pareview.sh/pareview/httpgitdrupalorgsandboxsupermario2328781git
Also I have mentioned the automated review link in issue summary page and also updated it.

You should first fix above issues first and get for coming in the admin radar you need do get review bonus by manual reviews of another 3 modules. This would help speed up the process and will put you on the high priority list, then git administrators will take a look at your project right away.

super_mario already said that the automated review lists potential false positives, so please do a real manual review.

Instead of maintaining your own database table, would it not be possible to build on top of core's file field? That would significantly reduce the amount of code you need here.

Hi @super_mario,
Thanks for your contribution.Nice module and also sub modules.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review

Yes/No: Follows the guidelines for project length and complexity.

Secure code

May be. Not reviewed complete module yet. If "no", list security issues identified.

Coding style & Drupal API usage

1. Update the changes as mentioned by @klausi in #6 https://www.drupal.org/node/2337931#comment-9172793
2. Also address on #14 https://www.drupal.org/node/2337931#comment-9188357
3. ixi.install : Set these values of the variables a default value instead of setting on the hook_install.Full specific as @klausi addressed. Do not set variables in hook_install() with variable_set(), you can just use default values with any variable_get() call
4. ixi.install : First check that whether the private filesystem path is set or not.
5. ixi.module : Rename the function name _ixi_config_form to ixi_config_settings_form as it seems more good.
6. ixi_upload.module : Change the pagecallback & page arguments of the 'admin/content/add/xml' path to be same as the 'admin/content/add/xml/upload'. No need to specify the redirect it will open the upload page callback same on the 'admin/content/add/xml'

This review uses the Project Application Review Template.

You should fix above issues first and get a review bonus by manual reviews of another 3 modules that would help speed up the process and make sure it gets on the review admins radar.

I got the Error during installation of modules so would you please fix this.Screenshot attached Due to this I was not able to test the module completely.

Thanks Again !

Screenshot as mentioned in #15

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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