Closed (won't fix)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
14 Jun 2016 at 05:54 UTC
Updated:
20 Jun 2016 at 16:12 UTC
Jump to comment: Most recent
Comments
Comment #2
PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgsandboxsugandhkhanna2743805git
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.
Comment #3
hiramanpatilHello sugandhkhanna2,
My findings for your module as follows:
1) There are some lines in README.md file which are exceeding 80 characters limit.
2) In email_token.module file, at line no 42 Closing brace indented incorrectly. It should have 2 spaces.
3) Remove "version" form the email_token.info.yml file, it will be added by drupal.org automatically.
Thanks
Comment #4
Anonymous (not verified) commentedHi sugandhkhanna2! First, I just wanted to say I like the idea of this module. I've often found the need for these values in emails but can see the value in not installing the entire Token module just to use them. Below are my findings while reviewing this module.
Token Filter Dependency:
In the description you point out that this module relies on the Token Filter module. Because of that, you'll want to make sure you specify that dependency in the "email_token.info.yml". This will make sure users have that module installed before yours can be enabled. The code for that will be something similar to what is below.
That being said, because the Token Filter module depends on the Token module being installed it would probably make more since for users to just use the Token module directly instead of installing these two additional modules
Automated Review Issues:
In order for this module to be promoted you'll need to run an automated review of the codebase using PAReview and fix the errors. I ran the review for this project that you can see here and there are a good amount of issues for you to work on.
Instructions
I saw that you took the time to create a help page which is awesome! Even with that though I was a little confused on where to actually put the tokens for them to be displayed in emails. Is this done within the settings for the Contact module or are they only available through custom code when manually building emails?
I hope this helps!
Comment #5
sugandhkhanna2 commented@keeganhr thanks for reviewing this module. I am working on the issues. And yes, you can put these tokens anywhere where text formats filters exists, like:
1. Basic html
2. Restricted html
3. Full html
And, I will surely work on the dependency which you have suggested.
Comment #6
sugandhkhanna2 commentedComment #7
yogeshmpawarPareview.sh is giving following errors.
http://pareview.sh/pareview/httpsgitdrupalorgsandboxsugandhkhanna2743805git
EDIT: removed long pareview.sh dump.
Will do a manual review in some time.
Comment #8
klausi@Yogesh Pawar: please don't copy & paste the long pareview.sh dump into a comment, the link should be enough or post the results as attachment.
Comment #9
sugandhkhanna2 commentedComment #10
sugandhkhanna2 commentedComment #11
sugandhkhanna2 commentedComment #12
sugandhkhanna2 commented