Closed (won't fix)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
13 May 2015 at 12:29 UTC
Updated:
3 Aug 2015 at 16:25 UTC
Jump to comment: Most recent
Comments
Comment #1
PA robot commentedWe 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
prajaankit commentedComment #3
PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxprajaankit2486011git
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #4
Vikas.Kumar commentedThere are some issue inside module info file as well you need to use proper commenting in module file for each function. Add readme .txt file for use condition for module.
Comment #5
rajesh.vishwakarma commentedAutomated Review
Your module have some error that reported by the automated system. Those need to fix.
http://pareview.sh/pareview/httpgitdrupalorgsandboxprajaankit2486011git
Manual Review
Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: It has additional integration with views also available statics in our system.
Master Branch
Yes: Follows the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements
3rd party assets/code
Yes: Follows the guidelines for 3rd party code.
README.txt/README.md
Yes: Follows the guidelines for in-project documentation
Code long/complex enough for review
No: Please check guidelines for project length.
Secure code
Yes: Meets the security requirements
Coding style & Drupal API usage
1) First fixed coding format as reported by http://pareview.sh/pareview/httpgitdrupalorgsandboxprajaankit2486011git
2) Replace core = 7.x-1.x with core = 7.x in image_alt_text.info file, due to this line module display This version is not compatible with Drupal 7.x and should be replaced. checked VERSION 7.34.
3) This module restrict for all images field required even where I don't need to make mandatory.
4) How can I make only single (title or alt) field required. There should be settings like check box with both field show that user can manage easily.
Comment #6
prajaankit commentedThank rajesh for your valueable comment
we remove all the coding standard error
Comment #7
prajaankit commentedComment #8
prajaankit commentedComment #9
prajaankit commentedComment #10
prajaankit commentedComment #11
prajaankit commentedComment #12
darol100 commented@prajaankit,
these are security issue. For this reason this is a blocker. You need to fix this.
Comment #13
dydave commentedHi guys,
Stumbled on the module at random.
First of all @prajaankit, Thanks a lot for making this step forward, towards contributing Drupal projects. That's the spirit! It's definitely greatly appreciated.
Without even going into PAReview, automated testing, security issues or code checking, I would have the following comments:
1 - Duplication
Isn't this module actually duplicating a feature/functions supposed to be part of the Accessible module?
See #887424: Make alt text required on imagefields and #815144-5: Imagefields should have the option to require Custom Alt text, also related to this feature: #2303765: Make the default 'alt' attribute for Image fields required and #347070: Ability to make alt and title text required for each image.
and what about the Extend Image Module?
It would have been nice if you could have given some details about where this requirement came from (related to an issue on Drupal.org, for example) and why it was decided to be developed as a module instead of a feature part of another module.
Wouldn't it be better to join your forces/efforts with the Accessible module (which by the way needs help and a co-maintainer) to get this pushed in the module as a patch, rather than duplicating the feature in another module...
which by the way doesn't even contain 120 lines of processing code/functions.... (2 - Code too short)
In my opinion, these 2 points above would seem to be critical enough to actually reject your project application.
Could you please bring a solid and robust justification of why this feature should be a stand-alone/independant module? Isn't there any other module it could be attached to/part of? Could you please share the results of your research with us (links, issues, etc...) whatever you found related with the duplication of this feature)?
3 - Project page needs improvement
On top of that, Image ALT text project page is really poor... I mean, two lines of information?! ... that's very very far from what would actually be expected for a module going through this validation process and best practices in general (if you would like anybody to actually really use your contributed modules).
If you are serious about contributing modules and becoming a vetted Drupal developer, I would strongly recommend that you take a closer look, at least once, at the following documentation:
My personal advice: drop this project application and try to get this pushed in Accessible as a co-maintainer.
Get a bit more practice as a co-maintainer and get back to project application after a while with another module, better prepared.
Just my 2 cents. Hope that helps, even though that's probably not what you want to hear.... I prefer to share my honest opinion.
Thanks again very much for your contributions, positive attitude and involvement in the Drupal developers community.
Cheers!
Comment #14
darol100 commentedRemoving the PAReview: security after some testing this does not see to be an security issue.
Comment #15
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.