This issue is focused on ensuring that the Drupal\Component\GetText directory is consistent with Drupal coding and documentation standards as well as other observations from reading through the various files to make this more consistent with the rest of Drupal core.
Among the items identified were the following:
a) The file docblock should use a description of 'Contains' instead of 'Definition of'. @file docblocks no longer used.
b) Private properties of classes vis-a-vis protected. Renamed to be protected so classes could be extended.
c) Same with private methods. Renamed to be protected so that classes can be extended if necessary.
d) Verb tense to start method docblock one-liners.
e) Completion of docblock @param, @return, @thows
The scope of this issue is broken down into:
- #2935189: Complete docblock comments in Gettext component.
- #2935190: Fix variable names in Gettext component.
- #2935192: Change 'private' to 'protected' in Gettext component.
- #2935193: Fix broken exceptions in Gettext component
The remaining documentation changes to locale module (out of scope for this issue) have been moved to #2935199: Add missing parameter and return documentation for Gettext.
Comment | File | Size | Author |
---|---|---|---|
#10 | drupal-gettext-review-1861442-10.patch | 62.25 KB | Sutharsan |
Comments
Comment #1
plachBetter component?
Comment #5
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedLets start with the documentation improvements
Comment #6
cilefen CreditAttribution: cilefen as a volunteer commentedComment #7
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedI realize that I mis-interpreted the scope of this issue. I started with Gettext related code in Locale module, this patch also includes the Drupal\Component\Gettext code. It has grown to be a fairly large patch, if splitting it is better for review and acceptance that is Ok with me.
This patch contains mostly docblock changes, but also changes:
-
return;
toreturn NULL;
-
$_this_property
to$thisProperty
- private properties and methods to protected
-
throw new Exception
tothrow new \Exception
Comment #8
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedThe new patch. No interdiff added (yet) due to the size of the second patch.
Comment #9
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedRe-roll.
Comment #10
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedRe-roll
Comment #11
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedI decided to split this issue into:
- Getttext and Locale
- Comment changes
- Rename variables (remove underscore prefix)
- To be determined
Back to needs work.
Comment #12
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedComment #13
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedComment #14
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedComment #15
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedComment #20
andypostIn #3236769-17: PoItem causes deprecation errors on PHP 8.1 there's concerns raised to improve
PoItem::setFromArray()
to prevent explode when no translation passedComment #21
andypost