Description :
Hi,
Developer Docs is a module specially made for drupal developers (amateur). When a new module is installed, it is hard to find what changes it does to the system unless we look into the code. This module provides an overview of "what it does" without looking into the code.
This Module generates a document automatically based on the drupal elements used in the module such as:
- Administration menu
- Hooks
- Database Schemas
- Themes
- Variables
- Blocks
- Node types
- Entity Types
- Fields (Formatter, Widget)
- Field Formatters
- Field Widgets
- Hooks Created
I hope this module will help the beginners of drupal development to climb the steep learning curve.
Sandbox Link:
https://www.drupal.org/sandbox/pravin.ajaaz/2467369
CLone URL:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/pravin.ajaaz/2467369.git developer_docs
Manual reviews of other projects:
- https://www.drupal.org/node/2384917#comment-9986451
- https://www.drupal.org/node/2498811#comment-9984429
- https://www.drupal.org/node/2498413#comment-9984093
- https://www.drupal.org/node/2498921#comment-9984027
- https://www.drupal.org/node/2491645#comment-9972111
- https://www.drupal.org/node/2486277#comment-9968305
- https://www.drupal.org/node/2495539#comment-9968273
- https://www.drupal.org/node/2429983#comment-9968289
- https://www.drupal.org/node/2496507#comment-9969277
- https://www.drupal.org/node/2490266#comment-9971937
Comment | File | Size | Author |
---|---|---|---|
#11 | Webform Module -Documentation .png | 167.27 KB | Pravin Ajaaz |
Comments
Comment #1
edutrul CreditAttribution: edutrul commentedHi pravin ajaaz,
according to http://pareview.sh/pareview/httpgitdrupalorgsandboxpravinajaaz2467369git-0
please fix the following:
FILE: /var/www/drupal-7-pareview/pareview_temp/developer_docs.module
---------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------
753 | WARNING | Only string literals should be passed to t() where
| | possible
---------------------------------------------------------------------------
Comment #2
edutrul CreditAttribution: edutrul commentedone suggestion for all your callbacks/your own functions is to add documentation @param... @return... see: https://www.drupal.org/coding-standards/docs
Comment #3
edutrul CreditAttribution: edutrul commentedComment #4
edutrul CreditAttribution: edutrul commentedAlso in lines 53-56 developer_docs.module
instead of using:
create a function to return a form and call it with drupal_get_form
Comment #5
edutrul CreditAttribution: edutrul commentedPlease refactor the above
Suggestion:
put what doesn't repeat in an array, e.g: hook_name, pres_elements index associative, widget_header(title & desc)
e.g:
Let me know if you have any questions, I't be a pleasure to help
Comment #6
edutrul CreditAttribution: edutrul commentedBTW, nice module!
Comment #7
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commented@edutrul: Thanks for the review and your kind words. I made the changes you suggested:
1. Pareview Warnings.
2. @param and @return for functions.
3. Used drupal_get_form instead of HTML tags.
4. Your suggestion to optimize the info hooks render arrays.
Comment #8
PA robot CreditAttribution: 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 #9
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedComment #10
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedComment #11
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedComment #12
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedComment #13
chenderson CreditAttribution: chenderson commentedAutomated Review
All is fine.
Manual Review
Two typos in developer-docs-modules.tpl.php
Line 22: Maintanance should be Maintenance
Line 44: I am not 100% sure but I think "Informations about..." should be "Information about...", I am unaware of any case for using the s in English.
Your code seems to be working as described. However my concern is the size of you functions, I notice two over 100 lines long. Your developer_docs_module_overview function seems like it could be refactored, however at a minimum I think you need to split up the develop_docs.module file.
I am unsure if file size and function size are part of any standard so you may want to wait for a second opinion before you change them.
Comment #14
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commented@chenderson: Thanks for looking into my code so close and pointing me the typos. I will fix those. As you told if the function size is defined in coding standards or if it affects the performance I will consider fixing them.
Comment #15
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedFixed all typos. Please consider moving the application to RTBC, if no major issue is found.
Comment #16
Ayesh CreditAttribution: Ayesh commentedLooks like a really nice module.
I will take a deep look tonight.
I read the code and found a few strong literals that are not wrapped with t() function. Not a blocker though.
I will write an in-depth review tonight.
You need to associate your git commits with your drupal.org account.
See https://www.drupal.org/node/1051722
Comment #17
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedHey hi ayesh,
I have been following your blog and stack answer's. I am so happy that you spent time looking into my code. Looking forward for your suggestion to make my module a standard one. :)
I will check the post regarding the git commit.
Comment #18
kandy-io CreditAttribution: kandy-io commentedHi Ajaaz!
I think this is a perfect code module. I use phpcs to verify your code and get this:
It's nothing important, but please change your end of line character from \r\n(windows) to \n (linux) if you continue developing your code.
Manual Review:
In developer_docs.module
Line 597:
$read_me = '';//should remove
Line 135: $render_contrib_hooks => $render_contribute_hooks //avoid error notify.
Line 243: $modulelist => $module_list //avoid error notify.
Line 611: $maint_status =>$main_status // ok?
Line 803: $doclink => $doc_link;//clear name
Comment #19
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commented@kandy-io: Thanks for the review. I made the changes you suggested regarding the variable name and I also fixed pareview warnings. I am using Linux style line endings in my text editor (sublime) already but I didn't know how do you get this warnings. Thanks for your time.
Comment #20
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedComment #21
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedComment #22
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedComment #23
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commented@ayesh: Is there any way to add this feature to drupal.org project pages. So that users can look at this overview before downloading a module and congratz on becoming a git administrator :)
Comment #24
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedHi reviewers, If there are no blockers then consider moving the project application to RTBC as soon as possible.
Comment #25
Ayesh CreditAttribution: Ayesh commentedHi Pravin,
Thank you for your wishes about the git admin role. It wouldn't be possible without the other senior admins and the welcome was quite warm.
Sorry about my delay in the review. But I could install and use it with many modules.
These are merely suggestions, and are in the order I found them. Nonetheless this is a really good module idea and worked out of the box quite well. None of the points below are release blockers, so I'm setting the status to RTBC.
- Module create date shows 1970/1/1, which indicates the UNIX timestamp must have been zero. Can we hide the date instead of showing the 1970 date. Also, consider using format_date to format the date.
- developer_docs_modules_list() - Output of this function could use renderable arrays. You can use
#theme => table
to theme tables, and (I know a previous reviewer mentioned the opposite before) the form could use textfield as#theme => textfield
. Advantage of drupal_get_form would be the ability to alter it (since it has no submit handlers or any validation), which a hook_page_alter will be able to do if we provide a complete renderable array.drupal_add_*
calls can be suppressed in favor of#attached
property. There are many modules that do not use renderable arrays, but I thought to mention this now that Drupal 8 uses more and more renderable arrays and it's logically the best way to build the content.- developer_docs_module_overview(): Output when the module is not enabled, the t() function contains a string literal with h4 tag. It would be better if no HTML is in
t()
function's string literal. But inline HTML elements (anchor tags for example) are rather encouraged because it gives translators the context.Also, instead of using
$base_path
to create URLs, you can useurl()
to build the URL.- A few typos (Admininstration -> Administration, etc).
- developer_docs_get_module_info() could use a few checks to make sure the file_get_contents() call was successful, filter_xss_admin() to make sure the README.txt file contents are safe and contains only proper HTML, etc.
- There are some hooks that do not return any data when called. Take block_block_info() for example. If there are no custom blocks, it will return an empty array. However, the way developer_docs_info_hooks() function collects hook information expects data to be there. Initializing and empty array right above the foreach loop should fix the problem. (Go to admin/reports/developer-docs/block page when all errors are enabled. You will see a PHP Notice for undefined variable
blocks
.- The module list table could use a unique HTML ID. Then, the developer_docs.js file could use that ID to filter rows. A page can contain many tables with
sticky-table
class, which can cause troubles with the JS added by this file. Also, it's a general practice to use Drupal behaviors to add event handlers. Make sure you use the.once()
notation ($(".elements").once('keysearch').keyup(...
).All above are just small issues, and not exactly blockers. Once again thank you for this, and lets see what others have to say.
Comment #26
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commented@ayesh: thanks for your valuable suggestions and your time. I did changes based on it.
1. developer_docs_module_overview output as a renderable array.
2. Text changes
3. developer_docs_get_module_info() - checks and filter
4. PHP warning notice fix
5. JS fixes.
6. used format_date().
Waiting for the final review :)
Comment #27
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedAdded three more manual reviews to speed up the process.
Comment #28
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedComment #29
Ayesh CreditAttribution: Ayesh commentedHi Pravin,
Thanks for the additional reviews. They are certainly helpful to maintain the queue, and shows you know the Drupal APIs and best practices quite well.
I think we should leave this application for a few days to let anyone who might have any other reviews to go ahead post them. I will proceed with the approval after a few days if there are no objections.
Comment #30
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commented@ayesh: Okay thanks.
Comment #31
Ayesh CreditAttribution: Ayesh commentedThanks for your contribution, Pravin Ajaaz!
I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.
Here are some recommended readings to help with excellent maintainership:
You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!
Thanks, 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.
Thanks to the dedicated reviewer(s) as well.
Comment #32
Ayesh CreditAttribution: Ayesh as a volunteer commentedComment #33
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedOMG!! Thanks ayesh and all the reviewers who reviewed my project. I'm so excited. I came here to contribute something back to the community but got much more in return. I will be in touch with you in issue queue :)