The sniff, Drupal.Array.Array.LongLineDeclaration is not enabled.
Enable Drupal.Array.Array.LongLineDeclaration in core/modules/*/Controller/*, core/modules/*/Database/*, and core/modules/*/Hook/* and core/modules/*/Controller/* 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.
Review
| Comment | File | Size | Author |
|---|
Show commandsStart 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:
changes, plain diff MR !14082changes, plain diff MR !13374
Comments
Comment #3
quietone commentedComment #4
quietone commentedComment #5
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #6
quietone commentedComment #7
smustgrave commentedLGTM!
Comment #8
quietone commentedOh 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.
Comment #9
longwaveSorry, another one that needs rebase.
Comment #10
quietone commentedRebased and ran phpcs. That found 2 new files that needed changes. So, the last commit should be reviewed.
Comment #11
smustgrave commentedHad to apply locally as the gitlab diff was hard to read but looked at https://git.drupalcode.org/project/drupal/-/merge_requests/13374/diffs?c... and seems fine to me
Comment #12
quietone commentedOn a self review I found that this had a commit for a different issue. I have reverted that and also removed an accidentally added blank line. Since the only change to the 'real' changes here was removing the blank line and linting is passing, I am leaving this at RTBC
Comment #13
longwaveSome more nits on this one to balance out brackets again.
Comment #14
quietone commented@longwave, thanks. I do appreciate the second opinion. I like the flexibility of sniff but sometimes it is difficult to know what other contributors will like.
I accepted the suggestions and rebased. So, I think it is fine to restore RTBC in this case.
Comment #15
quietone commentedAnother rebase dues to changes in phpcs.xml.dist. Leaving at RTBC
Comment #16
longwave11.3.x has a difference in content_moderation.module that means this doesn't apply cleanly there, not sure what to do about this but it would be nice to keep 11.3.x and 11.x as close as possible so cherry-picking fixes is easier...
Comment #18
quietone commentedI made an 11.3 MR and applied the diff and got failures in ContentTranslationFormHooks which only exists in 11.x. It looks like the form hooks were moved from ContentTranslationHooks to ContentTranslationFormHooks. It was only one line affected and when viewing it again I chose to improve that one change in both versions.
Comment #20
longwaveLet's get this in in the gap before 11.3.0 - no functional changes, just formatting.
Committed and pushed 673fd32689e to 11.x and a3460c4f797 to 11.3.x. Thanks!