I came across some coding style issues in working on issue #1701014: Validate config object names. Here's an issue to get them all cleaned up.

Remaining issues:

core/includes/config.inc:109

core/lib/Drupal/Core/Config/Config.php:98
core/lib/Drupal/Core/Config/Config.php:194
core/lib/Drupal/Core/Config/Config.php:196

core/lib/Drupal/Core/Config/FileStorage.php:47

core/modules/config/lib/Drupal/config/Tests/Storage/ConfigStorageTestBase.php:114
core/modules/config/lib/Drupal/config/Tests/Storage/ConfigStorageTestBase.php:116
core/modules/config/lib/Drupal/config/Tests/Storage/ConfigStorageTestBase.php:118
core/modules/config/lib/Drupal/config/Tests/Storage/ConfigStorageTestBase.php:120

CommentFileSizeAuthor
#9 1743072-file-content-test-cleanup-9.patch4.9 KBLinL
#6 1743072-file-content-test-cleanup-6.patch4.14 KBmtift
#3 1743072-file-content-test-cleanup-3.patch4.5 KBgdd
#1 drupal-1743072-fix_code_standards_config_system-1.patch15.69 KBdisasm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

disasm’s picture

disasm CreditAttribution: disasm commented
FileSize
drupal-1743072-fix_code_standards_config_system-1.patch15.69 KB

Attached is a patch handling the issues I found that ARE NOT handled above. Above issues still need to be resolved.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Active » Needs work
Issue tags: +Configuration system

Tagging.

Do we use "public" for all test methods now...? I'm not aware of that.

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
CreditAttribution: gdd commented
Status: Needs work » Needs review
FileSize
1743072-file-content-test-cleanup-3.patch4.5 KB

Almost all these cleanups have already been done, and I also don't believe that we're using public for test methods now. At least the other ones I spot checked aren't doing that.

So the only thing left is the small cleanups for ConfigFileContentTest which are all the rerolled patch is now.

disasm’s picture

disasm CreditAttribution: disasm commented
Issue tags: -Configuration system

#3: 1743072-file-content-test-cleanup-3.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Configuration system

The last submitted patch, 1743072-file-content-test-cleanup-3.patch, failed testing.

mtift’s picture

mtift
he/him
Primary language English
Location Minnesota, USA
CreditAttribution: mtift commented
FileSize
1743072-file-content-test-cleanup-6.patch4.14 KB

Here's a re-roll. Looks like a pretty straightforward fix.

mtift’s picture

mtift
he/him
Primary language English
Location Minnesota, USA
CreditAttribution: mtift commented
Status: Needs work » Needs review
mtift’s picture

mtift
he/him
Primary language English
Location Minnesota, USA
CreditAttribution: mtift commented
Status: Needs review » Needs work
Issue tags: +Novice, +Needs reroll
LinL’s picture

LinL CreditAttribution: LinL commented
Assigned: disasm » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1743072-file-content-test-cleanup-9.patch4.9 KB

Re-rolled. As before, plus I changed "an" to "a".

TR’s picture

TR CreditAttribution: TR commented
Status: Needs review » Reviewed & tested by the community

Looks fine. Almost all the changes are to comments. Let's commit this and move on.

TR’s picture

TR CreditAttribution: TR commented
Issue summary: View changes

Updated issue summary.

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented

9: 1743072-file-content-test-cleanup-9.patch queued for re-testing.

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented

9: 1743072-file-content-test-cleanup-9.patch queued for re-testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Nice clean-up.

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.