Closed (fixed)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
1 Apr 2019 at 09:07 UTC
Updated:
16 Apr 2019 at 18:29 UTC
Jump to comment: Most recent
Comments
Comment #2
avpadernoThank you for your contribution! I added the PAReview checklist. Reviewers will soon check the code and report here what they found out.
If you still haven't done it, please check what reported from the PAReview checklist. There could be some false positives; be careful of not changing code that should not be.
Comment #3
avpadernoComment #4
klausi* "variable_get('pdf_html_poppler_location', '/usr/local/bin')": all variables created by your module must be removed in hook_uninstall().
* "'#value' => 'Start',": all user facing strings must run through t() for translation. Please check all your strings.
* _pdf_html_config_form(): doc block is wrong, this is not a hook but a form building function. See https://www.drupal.org/docs/develop/standards/api-documentation-and-comm... . Same for the validate and submit callabck.
Otherwise looks good to me!
@kiamlaluno: I removed the single application approval tag, we have 330 lines of code here using various APIs, so that should be enough for general approval?
Comment #5
avpadernoThank you for your contribution! I am going to update your account.
These are some recommended readings to help with excellent maintainership:
You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.
I thank all the dedicated reviewers as well.
Comment #6
avpadernoI guess I put the tag in the wrong issue, or I checked the wrong branch. The code in this project is sufficient for this application.
Comment #7
arthur.baghdasar commentedThank you klausi and kiamlaluno for your help. Will review the code again.