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.
API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...
Enter a descriptive title (above) relating to Condition.php, then describe the problem you have found:
This class file has no documentation at the top of the class. Not even one line. Needs some.
This is a clear violation of the Core Gates for Documentation, so it is at least a major bug. Should be easy to fix -- there is a very similar class just one directory up.
Comment | File | Size | Author |
---|---|---|---|
#10 | 2191721-4.patch | 1.33 KB | divesh.kumar |
#8 | 2191721-3.patch | 2.81 KB | divesh.kumar |
#6 | 2191721-2.patch | 1.15 KB | divesh.kumar |
#2 | 2191721-1.patch | 1.39 KB | divesh.kumar |
Comments
Comment #1
jhodgdonAlso I think there are some old "Implements..." documented methods in this file that should be converted to inheritdoc.
Comment #2
divesh.kumar CreditAttribution: divesh.kumar commentedI have changed the documentation for it.
Comment #3
longwaveComment #4
jhodgdonThanks for the patch!
A class (or anything else) needs a one-line summary to start its documentation.
And for a class, this should start with a verb. We have standards on how to write documentation -- see
https://drupal.org/coding-standards/docs#classes
and this general section:
https://drupal.org/coding-standards/docs#drupal
The rest of the patch looks good.
Comment #5
divesh.kumar CreditAttribution: divesh.kumar commentedI have made changes as suggested.
Comment #6
divesh.kumar CreditAttribution: divesh.kumar commentedComment #7
jhodgdonOops, the one-line documentation intended for the class got into a method instead.
I also have a suggestion for the one-line class documentation... How about:
Implements entity query conditions for SQL databases.
(This is a class specifically for entity query conditions, and I think we need to get that into the documentation.)
Comment #8
divesh.kumar CreditAttribution: divesh.kumar commentedMy bad!!! Made changes in the new patch..
Comment #10
divesh.kumar CreditAttribution: divesh.kumar commentedComment #12
divesh.kumar CreditAttribution: divesh.kumar commentedPatch is failing due to LocalePluralFormatTest.php.
Comment #13
longwave10: 2191721-4.patch queued for re-testing.
Comment #15
jhodgdonThanks! Test failure is unrelated, and I don't think we need to hit "retest" as it's some "table not found" glitch in the test bot on one test. The patch looks good.
Comment #16
catchCommitted/pushed to 8.x, thanks!