Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The database driver task classes have missing or outdated documentation.
Comment | File | Size | Author |
---|---|---|---|
#18 | drupal_2084659_18.patch | 579 bytes | Xano |
#13 | drupal-2084659-13.patch | 4.44 KB | ju1iet |
#13 | interdiff.txt | 484 bytes | ju1iet |
#9 | drupal_2084659_9.patch | 4.41 KB | Xano |
#5 | interdiff.txt | 1008 bytes | Xano |
Comments
Comment #1
XanoComment #3
XanoComment #4
jhodgdonThanks! Good idea to fix up this documentation. A few issues in the patch:
a) This is not complete documentation: Return statements need a description.
b) This is not a documentation change -- if you mix documentation and non-documentation changes in a patch like this, I'll have to move it to "database system" and it will probably take more review etc. before it can be committed:
Please put these changes in a separate issue.
c) I don't see this on the base class that is being extended:
This occurs several times in the patch. It needs documentation and can't use inheritdoc unless there is documentation to inherit.
d) If you really want to clean up the documentaiton, most or all of the methods on the abstract base class do not follow our first-line verb coding standards (e.g., they should say "Checks..." not "Check"), and the class itself doesn't either. But if you want to not fix that in this patch, that is OK too.
Comment #5
XanoAbout the {@inheritdoc}, the patch adds code comments to the parent properties and methods.
Comment #6
jhodgdon#4 (a) still is a problem here. I'll review the patch once that is fixed.
Comment #7
XanoDo the coding standards require return value descriptions? I believe (and some other developers whose opinion I asked with me) that if an object of a particular interface or class is returned, unless there is additional context information, people should just look up the interface's or class's documentation.
Comment #8
jhodgdonYes, return value descriptions are required. Something simple is fine, like "The node that was added." for a function like node_add().
Comment #9
XanoComment #10
jhodgdonHow about for the @return description: A class defining the requirements and tasks for installing the database.
Everything else looks great.
Comment #11
jhodgdonComment #12
ju1iet CreditAttribution: ju1iet commentedComment #13
ju1iet CreditAttribution: ju1iet commentedComment #14
XanoRTBC, since the only change is a code comment, which cannot possibly break any tests.
Comment #15
alexpottCommitted 50ce328 and pushed to 8.x. Thanks!
Comment #16
jhodgdonQuick follow-up needed:
This has too many spaces in the line after @return. Should only be indented two spaces beyond the @ in the previous line.
Comment #17
XanoComment #18
XanoComment #19
jhodgdonThank you!
Comment #20
jhodgdonThanks Xano - committed to 8.x.