Problem/Motivation

The sniff, Drupal.Array.Array.LongLineDeclaration is not enabled.

Steps to reproduce

Proposed resolution

Enable Drupal.Array.Array.LongLineDeclaration in core/modules/*/Form/* directories.

There are no strict standards for how to split a call to a function across multiple lines. The changes try to be the best fit for readability and the current style used in the file or method.

Remaining tasks

Review Line 215 of \Drupal\system\Form\ThemeSettingsForm

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3549662

Command icon Show commands

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

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

About issue forks

Comments

quietone created an issue. See original summary.

quietone opened merge request !13369

quietone’s picture

quietone commented
Status: Active » Needs review
smustgrave’s picture

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

Seems straight forward and phpcs change would of caught any misses

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

This has commit conflicts.

smustgrave’s picture

smustgrave commented
Status: Needs work » Reviewed & tested by the community

I had a few minutes

        both modified:   core/modules/system/src/Form/ThemeSettingsForm.php
        both modified:   core/phpcs.xml.dist

First one was due to the theme_get_setting deprecation

quietone’s picture

quietone commented
Issue summary: View changes
Status: Reviewed & tested by the community » Needs review

I was checking all the MRs for related to this sniff because #3549652: Fix LongLineDeclaration in module Plugin directories was committed and the siblings will likely need a rebase.

There was a mistake in the MR where the change for phpcs.xml.dist made in #3549652: Fix LongLineDeclaration in module Plugin directories was removed as was the new line to be added here. Fixed that and ThemeSettingsForm. The change to ThemeSettingsForm was done by hand so setting this to needs review.

quietone’s picture

quietone commented

Oh dear, I was not working on HEAD and HEAD had a work in progress commit. I reverted that and pulled latest changes again, and then rebased this branch again. That brings this back to were it should be. There were no new conflicts and this was git work so leaving at RTBC. I could have just reverted that work in progress commit but for what ever reason I did not.

smustgrave’s picture

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

Hope it's fine I move back :)

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work

Made some suggestions to balance out brackets in various places.

quietone’s picture

quietone commented
Status: Needs work » Reviewed & tested by the community

Rebased and applied suggestions and re-checked the changes.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work

Sorry but I added some more nits.

quietone’s picture

quietone commented
Status: Needs work » Reviewed & tested by the community

No worries, I am glad these are getting reviewed.

Applied suggestions and linting passing so restoring RTBC

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I think this is ready to commit but granting credits is currently broken for me, will revisit once that is fixed.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 11.x-dev » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

Backported to 11.3.x as an eligible documentation fix.

Committed and pushed db51285f948 to 11.x and 8187276bc45 to 11.3.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

longwave closed merge request !13369

  • longwave committed 8187276b on 11.3.x 
    docs: #3549662 Fix LongLineDeclaration in module Form directories

By:...

  • longwave committed db51285f on 11.x 
    docs: #3549662 Fix LongLineDeclaration in module Form directories

By:...

Status: Fixed » Closed (fixed)

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