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.
Problem/Motivation
\Drupal\system\Tests\System\AdminTest doesn't document $admin_user or $web_user and has a non-standard @file
Proposed resolution
add documentation
Comment | File | Size | Author |
---|---|---|---|
#9 | interdiff-2262247-7-9.txt | 674 bytes | amitgoyal |
#9 | 2262247-9.patch | 869 bytes | amitgoyal |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin commentedSimple doc fixes.
Comment #2
tim.plunkettThat's true of dozens of other tests true...
Comment #3
pwolanin CreditAttribution: pwolanin commented@tim.plunkett - is it better to fix them in small patches when working on related code or to try to make one monster patch?
I was working on this test file for the menu links patch - putting this patch up so as to avoid including unrelated cleanup in that patch seems like the better pattern.
Comment #4
jhodgdonThis patch isn't quite right if you want to fix the standards:
===> Contains should be capitalized.
Comment #5
amitgoyal CreditAttribution: amitgoyal commentedThe patch in #1 was no longer getting applied on current code. So fixes for that and changes as per #4.
Comment #6
jhodgdonSorry, missed this in my last review. The class name here should start with \ at the beginning of the namespace.
Comment #7
amitgoyal CreditAttribution: amitgoyal commentedPlease review updated patch as per fix in #6.
Comment #8
jhodgdonSorry again... The @var lines should not end in $variable_name. See
https://drupal.org/node/1354#var
Comment #9
amitgoyal CreditAttribution: amitgoyal commentedSure and thanks for the feedback in #8. Please review updated patch.
Comment #10
jhodgdonOK, now we're OK. Thanks!
Comment #11
jhodgdonThanks again! Committed to 8.x.