Closed (won't fix)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
26 Apr 2014 at 23:36 UTC
Updated:
24 Mar 2015 at 11:24 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://pareview.sh/pareview/httpgitdrupalorgsandboxmscharl2251175git
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
mscharl commentedI'm sorry - I don't get this "review bonus thing".
I've checked my project with Pareview and fixed all warnings, errors, etc.
Comment #3
mscharl commentedComment #4
chtombleson commentedThe module works wells, but you need to keep the length of your lines down (Line: 43 in parsedown.module). 181 characters in one line is too much, try to keep lines to 80 characters if possible.
Comment #5
heddnConsider integrating https://drupal.org/project/libraries and most definately add a hook_requirements() (placed in your .install file).
Comment #6
mscharl commentedIntegrating https://drupal.org/project/libraries as a great idea - thank you
Comment #7
olivierg commented- I downloaded the last version of this module.
- I installed module libraries (It seems to be required now)
- I saved a configure text format with Parsedown.
- I put your readme fille in an article and, if i understood it would be transform in html ?
But for me it doesn't work.
Comment #8
sunnyuff commentedHi mscharl,
Nice module, works well.
While reviewing the module I found a few problems :
1. parsedown.module (Line #42) : You are adding HTML inside t(), which is a bad idea coz it will create problem in translation. Also you shouldn't concatinate strings inside t().
2. Please give proper info about where to keep the parsedown class otherwise many users will feel it difficult to install the module, first I read the desc given in module page which says it should be
, and then I got the readme which says
. I got confused and I checked the code and finally I found you are using libraries to load the class, that's nice but please update the docs.
3. This is my personal thinking, let me know what you think : in hook_requirements, you are only using runtime case, I think it will be good if you add a install case and probably add drupal_set_message() about the parsedown library if its not found. I have seen many users who dont want to check the readme and just want the module to work after install.
Just found another module Markdown Filter, which does exactly what we are doing here, can you please explain how your module is different from that. We prefer collaboration over competition, therefore we want to prevent having duplicating modules on drupal.org
Lastly, the review bonus thing :
Drupal is all about open-source and free contribution, but we need people who will review other projects, so we have create a rule where every new moudule contributor will require to review at least 3 other projects to get full project access in drupal.org
Useful links :
1. How to review Full Project applications
2. Project application checklist
3. Review bonus
4. Projects Needs Review
Thanks,
Comment #9
olivierg commentedHi sunnyuff,
I put Parsedown.php in the module folder but for me it's not working.
Comment #10
sunnyuff commentedHey Blobsmith,
So here are the instructions :
1. Download the module and add it in sites/all/modules/parsedown
2. Download Libraries module and enable it.
3. Download Parsedown PHP Library and add it in libraries, your complete path should look like sites/all/libraries/parsedown/Parsedown.php
4. Enable the Parsedown module.
5. Clear cache.
6. Navigate to "admin/reports/status" and search for (Parsedown Installed), means Libraries Module is successfully able to load the library.
7. Create a new node with text format parsedown and add parsedown markup,
Let me know if it now works for you.
Regards,
Comment #11
heddnparsedown_filter_tips()Use l() instead of creating a link manually in the t().
I agree with the earlier comment about concatenating strings into t().
parsedown_requirements($phase)Use placeholder replacement for $t(). That way you don't even need to add the tags for sites/all/libraries/parsedown/Parsedown.php.
Also, not all libraries are placed in sites/all/libraries. Sometimes they are in a distribution or in sites/default or sites/example.com.
Clarification: bonus review isn't required but highly encouraged.
However, not finding any security issues, license violations or 3rd party issues, I'm moving this to RTBC. The next step is to wait for a git administrator to review the project and grant git access.
Comment #12
olivierg commentedYes it works!!
Ok Good comment sunnyuff,
the documentation have to be modify about inclusion of the Parsedown.php library...
Comment #13
stborchertHi.
In my opinion there is one major issue with this project application: duplication.
Your approach sounds like a feature that should live in the existing Markdown project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the Markdown issue queue (or comment on #2288033: Consider a more modern markdown implementation?) to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.
If that fails for whatever reason please get back to us and set this back to "needs review".
Thanks.
Comment #14
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.