Closed (won't fix)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
13 May 2013 at 18:43 UTC
Updated:
27 Sep 2014 at 22:25 UTC
Jump to comment: Most recent
Comments
Comment #1
PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://ventral.org/pareview/httpgitdrupalorgsandboxiPerceptions1993844git
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 #2
neilsabharwal commentedComment #3
klausiPlease fix the most basic coding standard errors first: http://ventral.org/pareview/httpgitdrupalorgsandboxiPerceptions1993844git
Comment #4
iPerceptions commentedFILE: /var/www/drupal-7-pareview/pareview_temp/iperceptions_surveys.info
--------------------------------------------------------------------------------
FOUND 4 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
1 | ERROR | "name" property is missing in the info file
1 | ERROR | "description" property is missing in the info file
1 | ERROR | "core" property is missing in the info file
5 | ERROR | Files must end in a single new line character
-------------------------------------------------------------
That is the only error. It is wrong because these properties are clearly in the file.
Comment #5
klausiiperceptions_surveys.info vs. iperceptions_survey.info: why two info files?
Comment #6
iPerceptions commentediperceptions_survey.info has been deleted
Comment #7
iPerceptions commentedi deleted one of them. It should be fine now
Comment #8
iPerceptions commentedThe module should be working now, No issues with the automated review tool either
Comment #9
PA robot commentedClosing 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.
Comment #10
iPerceptions commentedOur understanding is that this has been fixed since May 14. How do we get this looked at again to be submitted?
Comment #11
abhishek-anand commentedHi iPerceptions,
Following are some of the issues that I could find out at the first look at your module.
1) In hook_help there are some strings which are not passed through t() function, Also the formatting is wrong. Kindly correct the formatting use spaces instead of tabs.
2) In iperceptions_survey.install line 12: inappropriate use of get_t
Correct usage:
3) Do we need to a drupal_set message in hook_install, instead add configure = admin/config/user-interface/iperceptions_survey in .info file
4) echo in hook_init() This is inappropriate. Use hook_page_build or hook_preprocess
Comment #12
iPerceptions commentedComment #13
iPerceptions commentedComment #14
iPerceptions commentedComment #15
rmn commentedHi
I would suggest using a tpl for your markup rather than writing it as a string in your function iperceptions_survey_help. Here's why I think it's important in my experience:
1. UI/Markup should always be separate from the server side logic.
2. Much better readability for another developer going through your code.
Also, the indentation breaks at various places in your code. Please review the errors in your code at pareview.sh.
Thanks
Raman
Comment #16
rmn commentedMarking again as Needs review, since this HTML is within help function - so not really a blocker.
Comment #17
tr commentedPlease clarify - is there a specific person who is working on this project or is this a company project?
All Drupal user accounts are supposed to be for individuals, not shared groups of people. Likewise, git access to the drupal.org repository is granted to individuals only, not to groups. You may give multiple users access to your project, but each user must have his or her own account on drupal.org, you should not be sharing an account.
Comment #18
cll.iperceptions commentedThis is a company project.
We have created Drupal user accounts for people who will work on the module.
The module has been updated as suggested by rmn in comment #15.
Please review the work.
Thank you.
Comment #19
Noe_ commentedThese aren't show stoppers but I'd like to mention it because you should change them:
- There is still a master branch present.
- There is a thumbs.db that shouldn't be here.
Then there is https://drupal.org/node/1993896#comment-8341305 which is only partially solved, see point 3.
I consider this last point about not using the appropriate API so I will leave this as Needs review.
Otherwise I would have set it to RTBC.
Comment #20
pingwin4egAll 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.
Comment #21
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.