Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
The taxonomy 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 |
---|---|---|---|
#6 | clean_up_taxonomy-2396717-6.patch | 37.32 KB | hussainweb |
#1 | clean_up_taxonomy-2396717-1.patch | 38.66 KB | hussainweb |
Comments
Comment #1
hussainwebComment #2
Mile23Good work on plowing through these issues.
Changing the array declarations is really out of scope, but not that big a deal.
You added $vocabulary to both TaxonomyTestBase and VocabularyUiTest, which is its subclass. I'd leave the one in TaxonomyTestBase and remove the one in VocabularyUiTest.
Comment #3
hussainwebResponding to comments in #2:
1. I know it is out of scope but I am touching all that I can. It looks concise and much better. Of course, there is no difference in functionality.
2.
VocabularyUiTest
extends\Drupal\taxonomy\Tests\TaxonomyTestBase
. The other$vocabulary
definition is actually in\Drupal\taxonomy\Tests\Views\TaxonomyTestBase
. They are different classes. This means we need to define$vocabulary
inVocabularyUiTest
as well.I am setting it back to needs review as there is nothing to do here. :)
Comment #4
Mile23OK, so checking with phpcs I find that the error of having under_score property names is fixed to camelCase. The $variables question is answered, and yes, you're right, they're different. The out-of-scope change isn't such a big deal, so RTBC the patch in #1.
Comment #5
alexpottThese are out-of-scope changes.
Comment #6
hussainwebThere is no interdiff as this was also a reroll. I removed all the changes with the new array syntax except where the lines are very different.
Comment #7
Mile23Thanks for sticking with it, @hussainweb. :-)
Patch in #6 fixes the camelCase errors, and also doesn't have out of scope array definition changes.
Comment #8
alexpottCommitted a658710 and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation to the issue summary.