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 text module uses Test class members with underscored names. Some examples are big_user, web_user and admin_user, but there could be others. According to our coding conventions, these should be renamed to bigUser, webUser and adminUser. In addition, some properties are undefined but should be.
See the parent issue #1811638: [meta] Clean-up Test members - ensure property definition and use of camelCase naming convention.
Beta phase evaluation
Issue category | Task, because this is a coding standards change. |
---|---|
Issue priority | Not critical because coding standard changes are not critical. |
Unfrozen changes | Unfrozen because it only changes automated tests. |
Disruption | There is no disruption expected from this sort of change. |
Comment | File | Size | Author |
---|---|---|---|
#22 | clean_up_text_module-2380411-22.patch | 3.28 KB | areke |
Comments
Comment #1
GPrince17 CreditAttribution: GPrince17 commentedComment #2
GPrince17 CreditAttribution: GPrince17 commentedComment #4
pwolanin CreditAttribution: pwolanin commentedLooks like you missed a usage in TextFieldTest.php line 115
Comment #5
cilefen CreditAttribution: cilefen commentedComment #6
cilefen CreditAttribution: cilefen commented@GPrince17: See the update to the parent issue.
Comment #7
cilefen CreditAttribution: cilefen commentedComment #8
cilefen CreditAttribution: cilefen commentedComment #9
hussainwebThe error was probably due to changing
web_user
towebUser
. I have changed it in the parent classStringFieldTest
which is in field module. It is not strictly in the scope of this ticket, but it is a small enough change rather than setting up dependencies to the issue for field module's ticket. We will just need to do a simple reroll depending on which goes in first. There is no issue yet for fixing these issues in the field module, which means we should be fine.Comment #10
subhojit777Code looks good. Marking this as RTBC for now.
Comment #11
subhojit777I am very sorry. This issue needs work.
Comment #12
tibbsa CreditAttribution: tibbsa commentedThat's odd. With this patch applied here, that command does not turn anything up for me.
The only thing I could think that might trip this up (though the regexp should keep it from happening) are things like this, but this is valid:
Are you sure you tested against a patched copy?
Comment #13
subhojit777Will recheck it again today. Was a bit sleepy when I was testing this :-)
Comment #14
tibbsa CreditAttribution: tibbsa commentedComment #15
areke CreditAttribution: areke commentedMarking as RTBC. As tibbsa suggested, I don't think subhojit777 had applied the patch before searching for the text.
Comment #16
cilefen CreditAttribution: cilefen commented@areke: Thanks for reviewing. I have not checked this yet. Did you make sure all properties are declared and documented?
Comment #17
cilefen CreditAttribution: cilefen commentedadminUser and webUser are not documented.
Comment #18
areke CreditAttribution: areke commentedAdded documentation as per #17.
Comment #19
tibbsa CreditAttribution: tibbsa commentedStrictly speaking this should probably have...
* @var \Drupal\user\UserInterface
... for each of the docblocks as well.
Comment #20
areke CreditAttribution: areke commentedAdded the fixes to the errors mentioned in #19.
Comment #21
tibbsa CreditAttribution: tibbsa commentedAlmost. :)
Coding standards - Indenting and Whitespace
Comment #22
areke CreditAttribution: areke commentedDid I get it this time? :P
Comment #23
tibbsa CreditAttribution: tibbsa commentedComment #24
alexpottCommitted 86ced51 and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation for to the issue summary.