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

Fix and issue patch for below config files:

action.settings.yml
views.view.test_bulk_form.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

Title: Make sure all YML files in action module has no type-casting to string. » Make sure all YML files in Action module has no type-casting to string.
vijaycs85’s picture

Status: Active » Needs review
FileSize
4.82 KB

Initial patch...

vijaycs85’s picture

changing int to bool...

cosmicdreams’s picture

+++ b/core/modules/action/tests/action_bulk_test/config/views.view.test_bulk_form.ymlundefined
@@ -22,35 +22,35 @@ display:
+          override: TRUE

In other patches you use lowercase for boolean statements.

Which should they be uppercase or lowercase. Pick one.

tim.plunkett’s picture

Uppercase.
EDIT: chx's point below trumps all other conventions.

chx’s picture

lowercase. Bring the issue up with the symfony yaml guys. It dumps lowercase.

krishnan.n’s picture

'0' => false, '1' => true. This is only for: config/views.view.test_bulk_form.yml. Will work thru the rest, if this is OK.

krishnan.n’s picture

Please provide review feedback on the patch above, and i'll script through the rest of the tree -- thx.

chx’s picture

Yes this looks good! Careful with scripting. Perhaps share the script with us? You can't wholesale search-replace 1 and 0 to true and false, sometimes they are numbers.

krishnan.n’s picture

Script attached:

#!/bin/sed -f
# convert 0 to false and '1' to true, and also lower-case TRUE/FALSE
# Usage: boolconvert.sed <yml-file-name>

# searching for :<space><tab>(whole-word) '1' or '0'
# whole-word reason: we don't want to mess with '-1', 100, etc
# Bug: "token: 1 black sheep" will become "token: true black sheep"

s/:[   ]*'\<1\>'/: true/
s/:[   ]*'\<0\>'/: false/
s/:[    ]*\<0\>/: false/
s/:[    ]*\<1\>/: true/
s/:[    ]*FALSE/: false/
s/:[    ]*TRUE/: true/

Also, how best to walk through directory tree to pick up relevant yml files? This is the rough version:

# Pick up config files in ./modules/<module-name>/config/*.yml
#
find .  -name '*.yml' | grep "\/modules\/\<[a-z]*\>\/config\/.*yml\>"  | xargs sed -i -f ../boolconvert.sed

Better to do the above by hand?

damiankloip’s picture

Status: Needs review » Needs work
+++ b/core/modules/action/tests/action_bulk_test/config/views.view.test_bulk_form.yml
@@ -81,57 +81,57 @@ display:
-          group: '1'
...
+          group: true

Group should definitely be a number, this is the group used when the condition groups are built for the query. So 1 is the default group here.

damiankloip’s picture

Issue summary: View changes

updating views.view.test_bulk_form.yml

vijaycs85’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
4.67 KB

Re-roll...

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.