Closed (won't fix)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
19 Jan 2016 at 11:21 UTC
Updated:
22 Nov 2018 at 09:08 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxZohoCampaigns2545074git
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.
Comment #3
ZohoCampaigns commentedIssue fixed
Comment #4
jlicht commentedManual Review
Individual user account
No: Does not follow the guidelines for individual user accounts.
It seems you are using a non-individual account.
All user accounts are for individuals. Accounts created for more than one user or those using anonymous mail services will be blocked when discovered (see Get a Drupal.org account).
Please note that organization accounts cannot be approved for git commit access. See https://drupal.org/node/1966218 and https://drupal.org/node/1863498 for details on what is/isn't allowed. Please update your user profile so that we don't have to assume that this is a group account.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
Master Branch
Yes: Follows the guidelines for master branch.
Licensing
I’m not sure if it follows the licensing requirements (see note re: 3rd party assets below)
3rd party assets/code
No: Does not follow the guidelines for 3rd party assets/code.
[*] It looks like javascript files have been copied from Drupal core (tabledrag.js) and the slider field module (alter1.js) and included in this project
[*] The jquery_nicescroll module includes the jQuery.nicescroll.js javascript file which has been included in this module. Why not depend on that module instead?
README.txt/README.md
No: Does not follow the guidelines for in-project documentation and/or the README Template.
- Please take a moment to make your README.txt follow the guidelines for in-project documentation.
- List all required dependencies (e.g. modules that are listed as dependencies in the .info file).
- Note that CURL is required
- There is no need to provide instructions for installing Drupal or generic instructions for installing the module - you can link to standard Drupal documentation and just provide information about installation steps specific to your module
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
Difficult to review this until other issues addressed.
Coding style & Drupal API usage
General
- There are many issues reported by the automated review (see attached file)
- There is a great deal of CSS embedded in the PHP code. This should be pulled out into CSS files.
- Use descriptive variable names, and include some comments in the code.
- The CSS in the code, and lack of consistent formatting and indenting make this code very difficult to review (as well as the lack of comments).
- Consider splitting the .admin.inc into separate files (it is over 5500 lines of code)
ZohoCampaigns.admin.inc
- Do not assume the module will be installed in a specific location when constructing image paths (example: L3428)
ZohoCampaigns.install
[+] Namespace your variable names and table names by prefixing with the module name to avoid collisions with other modules.
ZohoCampaigns.module
[+] Do not set the global error_reporting value (L4)
[+] Use {table} syntax when running database queries (example: L856)
Comment #5
PA robot commentedClosing 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.
Comment #6
ZohoCampaigns commentedIssues have been fixed.
We are modifying the CSS property dynamically , hence it is not possible to pull it out to a seperate CSS file.
Comment #7
ZohoCampaigns commentedComment #8
warped commentedPlease do not assign this issue to yourself. Then it looks like changes are in progress.
Comment #9
avpadernoTo the reviewers: Please set the priority to Normal after reviewing the project.
Comment #10
sleitner commentedAutomated Review
Pareview details: https://pareview.sh/pareview/https-git.drupal.org-sandbox-zohocampaigns-...
Review of the 7.x-1.x branch (commit 5fa0439):
hook_help(). See https://www.drupal.org/docs/develop/documenting-your-project/module-docu... .Manual Review
Do not assume the module will be installed in a specific location when constructing image paths (example: L3428)
Namespace your variable names and table names by prefixing with the module name to avoid collisions with other modules.
The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.
If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.
This review uses the Project Application Review Template.
Comment #11
avpadernoComment #12
sleitner commentedComment #13
avpaderno27 commits have been done using dhivya-3115 as Git username, against 35 done from the user who is applying here. In this case, the project used to apply needs to be another one.
Since the account seems to be company account, though, the application cannot be accepted.
Comment #14
avpadernoI am closing this application as per my previous comment, and the lack of activity.