Follow-up from #1653026: [META] Use properly typed values in module configuration.

Problem/Motivation

All integers, Booleans, and even octal numbers in config object files are converted to strings.

Proposed resolution

#1653026: [META] Use properly typed values in module configuration has fixed core, so no need to convert all data types to string anymore.

Remaining tasks

system.authorize.yml
system.cron.yml
system.date.yml
system.date_format.fallback.yml
system.date_format.html_date.yml
system.date_format.html_datetime.yml
system.date_format.html_month.yml
system.date_format.html_time.yml
system.date_format.html_week.yml
system.date_format.html_year.yml
system.date_format.html_yearless_date.yml
system.date_format.long.yml
system.date_format.medium.yml
system.date_format.short.yml
system.file.yml
system.image.gd.yml
system.maintenance.yml
system.menu.account.yml
system.menu.admin.yml
system.menu.footer.yml
system.menu.main.yml
system.menu.tools.yml
system.module.yml
system.performance.yml
system.rss.yml
system.site.yml
system.theme.global.yml
system.theme.yml

entity_test/config/entity.view_mode.entity_test_render.full.yml
entity_test/config/entity.view_mode.entity_test_render.test.yml
entity_test/entity_test.routing.yml
update_script_test/config/update_script_test.settings.yml

User interface changes

NO

API changes

NO

Parent: #1653026: [META] Use properly typed values in module configuration

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vijaycs85’s picture

Issue summary: View changes

updating files list.

vijaycs85’s picture

Status: Active » Needs review
FileSize
59.77 KB

Initial patch...

Status: Needs review » Needs work

The last submitted patch, 2105993-system-yml-type-casting-1.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
12.92 KB

removing route 'TRUE' => TRUE changes..

Status: Needs review » Needs work

The last submitted patch, 2105993-system-yml-type-casting-3.patch, failed testing.

vijaycs85’s picture

Title: Make sure all YML files in System module has no type-casting to string. » Make sure all Config yml files in System module has no type-casting to string.
FileSize
12.11 KB

Removing routing changes, as advised by @timplunkett on IRC:

00:05 vijaycs85: timplunkett: does #1653026: [META] Use properly typed values in module configuration fix our accessChecker 'TRUE' to TRUE or true? seems, doesn't :(
00:05 timplunkett: vijaycs85: so routing is NOT config
00:06 timplunkett: and has nothing to do with that issue
00:06 timplunkett: vijaycs85: also, symfony's routing system enforces that requirement values are strings
00:06 timplunkett: vijaycs85: but that issue should only deal with stuff in the /config directory, nothing top level
00:07 vijaycs85 has to change the title of all sub-tasks at https://drupal.org/node/1653026#sub-tasks

vijaycs85’s picture

Status: Needs work » Needs review
vijaycs85’s picture

Issue summary: View changes

Updated issue summary with test modules

Status: Needs review » Needs work

The last submitted patch, 2105993-system-yml-type-casting-5.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
5.02 KB

Re-roll...

Status: Needs review » Needs work

The last submitted patch, 2105993-system-yml-type-casting-8.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
8.37 KB
13.39 KB

This change worth for a separate issue. Fixed all places where we did type casting (octdec() to convert string to oct) for file permission. Just love it testbot :)

YesCT’s picture

I'm reviewing this.

YesCT’s picture

The issue summary lists more files than the patch changed.

for patch in #10
git diff --name-only
gives:

core/includes/file.inc
core/modules/simpletest/simpletest.module
core/modules/system/config/system.date.yml
core/modules/system/config/system.file.yml
core/modules/system/config/system.image.gd.yml
core/modules/system/config/system.maintenance.yml
core/modules/system/config/system.module.yml
core/modules/system/config/system.performance.yml
core/modules/system/config/system.theme.global.yml
core/modules/system/config/system.theme.yml
core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php
core/modules/system/lib/Drupal/system/Tests/File/FileTestBase.php
core/modules/system/lib/Drupal/system/Tests/File/UnmanagedCopyTest.php
core/modules/system/lib/Drupal/system/Tests/File/UnmanagedMoveTest.php
core/modules/system/lib/Drupal/system/Tests/File/UnmanagedSaveDataTest.php
core/modules/system/tests/modules/entity_test/config/entity.view_mode.entity_test_render.full.yml
core/modules/system/tests/modules/entity_test/config/entity.view_mode.entity_test_render.test.yml
core/modules/system/tests/modules/update_script_test/config/update_script_test.settings.yml

For example, listed in the issue summary, but not changed in the patch is

system.rss.yml

----
Also, how was the list in the remaining tasks arrived at?

find core/modules/system -type f -name "*.yml" | wc -l

yields 133

an example: core/modules/system/tests/modules/twig_extension_test/twig_extension_test.services.yml

oh, services.. and routes and local tasks are not config (or at least not config we are looking at here)

find core/modules/system -type f -name "*.yml" | grep -v local_tasks.yml | grep -v services.yml | grep -v routing.yml | grep -v local_actions.yml | wc -l
reduces to 99

find core/modules/system -type f -name "*.yml" | grep -v local_tasks.yml | grep -v services.yml | grep -v routing.yml | grep -v info.yml | wc -l
reduces to 40

ok. so.

I put the list from the issue summary into a file, and took off the dir names from the few at the end.
Also redirected the output of the grepped find.
find core/modules/system -type f -name "*.yml" -exec basename {} \; | grep -v local_tasks.yml | grep -v services.yml | grep -v routing.yml | grep -v info.yml | grep -v local_actions.yml > grepdlist.txt

Then diff'd those two files.

diff issuesummarylist.txt grepdlist.txt

0a1,2
> system.data_types.schema.yml
> system.schema.yml
15a18
> system.filter.yml
16a20,23
> system.image.yml
> system.language.types.yml
> system.logging.yml
> system.mail.yml
22a30
> system.menu.yml
28a37
> config_upgrade.test.yml
31d39
< entity_test.routing.yml

filenames pointing to the left are in the summary,
pointing to the right are from the find

attaching also the output with the full paths.

Now, which from the find list are not touched by this patch?

cat patchchanged.txt |awk -F/ '{print $NF}' > patchchanged-nopath.txt

diff patchchanged-nopath.txt grepdlist.txt

1,2c1,4
< file.inc
< simpletest.module
---
> system.data_types.schema.yml
> system.schema.yml
> system.authorize.yml
> system.cron.yml
3a6,16
> system.date_format.fallback.yml
> system.date_format.html_date.yml
> system.date_format.html_datetime.yml
> system.date_format.html_month.yml
> system.date_format.html_time.yml
> system.date_format.html_week.yml
> system.date_format.html_year.yml
> system.date_format.html_yearless_date.yml
> system.date_format.long.yml
> system.date_format.medium.yml
> system.date_format.short.yml
4a18
> system.filter.yml
5a20,23
> system.image.yml
> system.language.types.yml
> system.logging.yml
> system.mail.yml
6a25,30
> system.menu.account.yml
> system.menu.admin.yml
> system.menu.footer.yml
> system.menu.main.yml
> system.menu.tools.yml
> system.menu.yml
8a33,34
> system.rss.yml
> system.site.yml
11,15c37
< DirectoryTest.php
< FileTestBase.php
< UnmanagedCopyTest.php
< UnmanagedMoveTest.php
< UnmanagedSaveDataTest.php
---
> config_upgrade.test.yml

So files pointing to the left are changes to files other than the config files found by the grep'd find,
files pointing to the right are not changed by this patch but are in the grep'd find.

========
A next step: figuring out why the grep'd find list of 40 files is different than those listed in the issue summary.
Another next step: looking at those (that were even listed in the issue summary) and seeing if they really need no change.

YesCT’s picture

Issue summary: View changes

Updated issue summary removing *.routing.yml files.

vijaycs85’s picture

Issue summary: View changes
FileSize
12.9 KB

Re-rolling...

vijaycs85’s picture

Status: Needs review » Closed (duplicate)
Related issues: +#2167623: Add test for all default configuration to ensure schema exists and is correct

The patch on this issue has been updated as part of #2167623: Add test for all default configuration to ensure schema exists and is correct. As this issue doesn't have any test to confirm/validate the schema, making this change and closing this issue as duplicate of #2167623: Add test for all default configuration to ensure schema exists and is correct. The contributors of this issue (in commit message) is copied to #2167623: Add test for all default configuration to ensure schema exists and is correct.