Remove quotes from olivero.info.yml [#3278144]

Problem/Motivation

In Olivero's olivero.info.yml file, some strings are quoted, others are not:

regions:
  header: Header
  primary_menu: 'Primary menu'
  secondary_menu: 'Secondary menu'
  hero: 'Hero (full width)'
  highlighted: Highlighted
  breadcrumb: Breadcrumb
  social: Social Bar
  content_above: Content Above
  content: Content
  sidebar: 'Sidebar'
  content_below: 'Content Below'
  footer_top: 'Footer Top'
  footer_bottom: 'Footer Bottom'

From https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/themes/olive...

The rules for strings in YAML are complicated, but since you don't need to quote a string, as long as the first character is not a special character, maybe we should just remove them for consistency? For more, see:

Proposed resolution

Remove all single quotes in olivero.info.yml.

Remaining tasks

Decide if removing all quotes is a good idea, or if alternatively all strings should be quoted?
Update Creating sub-themes documentation page, depending on the decision.

User interface changes

API changes

Data model changes

Release notes snippet

Comment	File	Size	Author
#28	Screenshot 2023-03-27 at 3.11.29 PM.png	177.73 KB	Santosh_Verma
#28	Screenshot 2023-03-27 at 3.08.23 PM.png	187.75 KB	Santosh_Verma
#26	after-patch-apply.png	69.34 KB	Akshay kashyap
#22	3278144-22.patch	1.21 KB	Rinku Jacob 13
#22	10.1.x: PHP 8.1 & MySQL 5.7 29,062 pass
#22	3278144-22.patch	1.21 KB	Rinku Jacob 13
#22	10.1.x: PHP 8.1 & MySQL 5.7 29,058 pass, 1 fail
#20	3278144-20.patch	890 bytes	Akshay kashyap
#20	10.1.x: PHP 8.1 & MySQL 5.7 29,061 pass, 1 fail
#15	3278144-15.patch	1.21 KB	Gauravvvv
#15	10.1.x: PHP 8.1 & MySQL 5.7 29,060 pass, 1 fail

Issue fork drupal-3278144

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

1 hidden branch

3278144-quote-names-in

changes, plain diff MR !2205

Check out this branch

About issue forks

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

1 May 2022 at 14:42

ressa created an issue. See original summary.

Comment #2

1 May 2022 at 14:47

ressa opened merge request !2205

Comment #3

ressa CreditAttribution: ressa at Ardea commented 1 May 2022 at 14:53

Issue summary:

View changes

Comment #4

ressa CreditAttribution: ressa at Ardea commented 1 May 2022 at 15:02

Title:	Quote names in regions section	» Quote names in regions section, perhaps also name, type, and package strings
Issue summary:	View changes

Comment #5

ressa CreditAttribution: ressa at Ardea commented 1 May 2022 at 18:22

Title:	Quote names in regions section, perhaps also name, type, and package strings	» Remove quotes from olivero.info.yml
Issue summary:	View changes

Comment #6

ressa CreditAttribution: ressa at Ardea commented 1 May 2022 at 20:16

Issue summary:

View changes

Comment #7

quietone CreditAttribution: quietone at PreviousNext commented 2 May 2022 at 04:22

Just noting that there are no Drupal coding standards for yaml files. I recall reading in some issue somewhere that the yml in that issue was quoted in the same way as exported from configuration management. I have not tested that.

I am curious to know why this was tested against so many environments.

Comment #8

Chi CreditAttribution: Chi commented 2 May 2022 at 04:57

Comment #9

ressa CreditAttribution: ressa at Ardea commented 2 May 2022 at 11:29

Thanks for the clarification @quietone, and @Chi for the issue link, I have added a comment there about expanding YAML string coding standards.

I am also puzzled by all the tests. It looks like they were triggered 2 May 2022 at 04:18 CEST by manojkumar1997 ...

The failing test in the "main test" PHP 8.1 & MySQL 5.7 29,741 is the golden oldie false positive from QuickEdit: "WebDriver\Exception\UnknownError: element click intercepted: Element ... is not clickable at point" I also posted about in https://www.drupal.org/project/drupal/issues/3227033#comment-14501046.

Comment #10

ressa CreditAttribution: ressa at Ardea commented 2 May 2022 at 13:35

All test options also seem to have been activated by this user in these two issues:

Comment #11

ressa CreditAttribution: ressa at Ardea commented 2 May 2022 at 18:23

Actually, it has been a blessing in disguise that all the tests were triggered, since it shows the randomness of the tests, and all the false positives it gives.

I have been trying to get the test to pass, and not break with a Quickedit false positive (third run now).

Pass

PHP 7.3 & PostgreSQL 10.12
PHP 7.4 & MySQL 5.7
PHP 8.1 & pgsql-10.12

Fail

Quickedit.Quickedit

WebDriver\Exception\UnknownError: element click intercepted: Element ... is not clickable at point

PHP 7.3 & MariaDB 10.3.22
PHP 7.3 & MySQL 5.7
PHP 7.3 & SQLite 3.28
PHP 8.0 & MySQL 5.7
PHP 8.1 & MySQL 5.7

Ckeditor5.Ckeditor5

Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testViewMode with data set "with alignment"

PHP 7.4 & SQLite 3.27
PHP 8.1 & sqlite-3.27

Comment #12

17 May 2022 at 20:54

ressa closed merge request !2205

Comment #13

17 May 2022 at 20:54

ressa reopened merge request !2205

Comment #14

17 May 2022 at 20:54

Version:

9.5.x-dev

» 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #15

Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commented 17 March 2023 at 12:49

Status:

Active

» Needs review

File	Size
3278144-15.patch	1.21 KB
10.1.x: PHP 8.1 & MySQL 5.7 29,060 pass, 1 fail

I have removed the quotes from olivero.info.yml file. please review

Comment #16

smustgrave CreditAttribution: smustgrave at Mobomo commented 17 March 2023 at 15:03

Status:	Needs review	» Reviewed & tested by the community
Issue tags:		+Needs Review Queue Initiative

Tested MR 2205 as it still applies to 10.1 and quotes are removed.

Comment #17

23 March 2023 at 17:27

Status:

Reviewed & tested by the community

» Needs work

The last submitted patch, 15: 3278144-15.patch, failed testing. View results

Comment #18

23 March 2023 at 18:53

ressa closed merge request !2205

Comment #19

ressa CreditAttribution: ressa at Ardea commented 23 March 2023 at 18:57

I attempted rebasing, but there were too many changes, after the failing test.

If I click "Create new branch", it won't let me create a new branch, I get a "Failed to create branch '3278144-remove-quotes': invalid reference name '10.1.x'" error ...

Update: See #3323320: Can not create new branches on issue forks 10.1.x branch.

Comment #20

Akshay kashyap CreditAttribution: Akshay kashyap as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented 27 March 2023 at 06:13

File	Size
3278144-20.patch	890 bytes
10.1.x: PHP 8.1 & MySQL 5.7 29,061 pass, 1 fail

Providing Patch

Comment #21

Akshay kashyap CreditAttribution: Akshay kashyap as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented 27 March 2023 at 06:14

Status:

Needs work

» Needs review

Comment #22

Rinku Jacob 13 CreditAttribution: Rinku Jacob 13 commented 27 March 2023 at 06:46

File	Size
3278144-22.patch	1.21 KB
10.1.x: PHP 8.1 & MySQL 5.7 29,058 pass, 1 fail
3278144-22.patch	1.21 KB
10.1.x: PHP 8.1 & MySQL 5.7 29,062 pass

Hi @Akshay kashyap, in your patch you have removed quotes from regions. But we have to remove quotes from description and alt_text key.Created new patch for that. Please review.

Comment #23

Rinku Jacob 13 CreditAttribution: Rinku Jacob 13 commented 27 March 2023 at 06:47

1 file was hidden/shown/deleted

File	Size
3278144-22.patch	1.21 KB
10.1.x: PHP 8.1 & MySQL 5.7 29,062 pass

File	Size
after-patch-apply.png	69.34 KB

I have reviewed patch #22. Quotes removed from olivero.info.yml please see the screenshot

Comment #27

Rinku Jacob 13 CreditAttribution: Rinku Jacob 13 commented 27 March 2023 at 07:52

1 file was hidden/shown/deleted

File	Size
3278144-22.patch	1.21 KB
10.1.x: PHP 8.1 & MySQL 5.7 29,062 pass

Comment #28

Santosh_Verma CreditAttribution: Santosh_Verma at Srijan | A Material+ Company for Drupal India Association commented 27 March 2023 at 09:44

File	Size
Screenshot 2023-03-27 at 3.08.23 PM.png	187.75 KB
Screenshot 2023-03-27 at 3.11.29 PM.png	177.73 KB

Applied patch #27 cleanly. quotes removed

Before
before

after
after

Comment #29

Santosh_Verma CreditAttribution: Santosh_Verma at Srijan | A Material+ Company for Drupal India Association commented 27 March 2023 at 09:45

Status:

Needs review

» Reviewed & tested by the community

Comment #30

27 March 2023 at 12:02

lauriii committed ae8b400b on 10.1.x

Issue #3278144 by ressa, Rinku Jacob 13, Akshay kashyap, Gauravvvv:...

Comment #31

lauriii

he/him

Finnish

Finland

CreditAttribution: lauriii at Acquia commented 27 March 2023 at 12:03

Status:

Reviewed & tested by the community

» Fixed

Reviewed this with @nod_, @justafish, and @ckrina!

Committed ae8b400 and pushed to 10.1.x. Thanks!

Comment #32

ressa CreditAttribution: ressa at Ardea commented 27 March 2023 at 22:16

Thanks! Nice to see Claro also getting cleaned up in #3350058: Remove quotes from claro.info.yml.

Comment #33

10 April 2023 at 22:19

Status:

Fixed

» Closed (fixed)

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

Remove quotes from olivero.info.yml

Problem/Motivation

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3278144

Comments

Pass

Fail

Quickedit.Quickedit

Ckeditor5.Ckeditor5

Related issues

Referenced by