Problem/Motivation
These lines are too long and should be broken up:
'my_module_display' => [
'variables' => ['my_modules' => NULL, 'topics' => NULL, 'parents' => NULL, 'tid' => NULL, 'sortby' => NULL, 'my_module_per_page' => NULL],
],
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|
Issue fork drupal-3471544
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:
- 3471544-array-lines-in
compare
- drupal-3471544
changes, plain diff MR !9391
Comments
Comment #4
joachim commentedRemember to set an issue to 'Needs review' when it's ready.
Looks good to me. Thanks!
Comment #5
ronttizz commentedThanks for the reminder. I'm trying to figure out why the tests are not passing. If you have any idea please let me know.
Comment #6
joachim commentedFunctional Javascript tests can be a bit temperamental. It can't be a problem with your MR, as api.php code is not executed.
Comment #7
kleiton_rodrigues commentedJavaScript tests can be a bit unpredictable.
The code changes in
core/lib/Drupal/Core/Render/theme.api.phpshould not directly affect these tests, I am investigating to better understand the issue.Comment #8
quietone commented@kleiton_rodrigues, thanks for the interest in this issue. In Drupal we do not upload images of our IDE, images are reserved for discussing changes to the User Interface. The failing test may be a random one. You can check by first reading #2829040: [meta] Known intermittent, random, and environment-specific test failures and there are instructions for how to deal with a random failure. The previous comment also did say that the failing test is not related to the change here.
Changing to a task because this is neither "Incorrect or misleading user interface text" or "Incorrect or misleading documentation"
Comment #9
joachim commentedIt's a bug because if people copy this code as a sample, then their code will fail coding standard.
Comment #10
longwaveLet's change my_module_list's variables to match while we are here, it's much easier to read in multiline format.
Comment #11
joachim commented+1, though 'my_module_icon' as well then too.
Comment #13
sonyavpaa commentedSeperated also the my_module_list and my_module_icon arrays into multiple lines, needs review!
Comment #14
smustgrave commentedSeems pretty straight forward.
Comment #19
quietone commented@joachim, I do get your point, but I am following the guidelines for the category field which does not mention copying example code.
Taking a more complete look at this now and this is coding standards issue, so tagging. The standard is Drupal.Arrays.Array.LongLineDeclaration", which is not yet enabled in core. The meta for to do that is #3116859: [meta] Fix Drupal.Array.Array.LongLineDeclaration coding standard. We have a practice of not accepting fixes for coding standards by file because it is not an efficient use of our time. That makes this a won't fix and the work here transferred to a child of the Meta.
On the other hand we are here and another committer has already expressed an opinion here and missed (like me) that this should be moved to the relevant meta. Therefore, I will commit this as a one off that is not meant to set a precedence. To use our time wisely coding standards fixes need to be done by sniff.
Committed/pushed to 11.x and cherry-picked back through to 10.3.x, thanks!