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

CommentFileSizeAuthor
#7 issues_3471544_7.png36.73 KBkleiton_rodrigues

Issue fork drupal-3471544

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:

Comments

joachim created an issue. See original summary.

ronttizz made their first commit to this issue’s fork.

joachim’s picture

Status: Active » Reviewed & tested by the community

Remember to set an issue to 'Needs review' when it's ready.

Looks good to me. Thanks!

ronttizz’s picture

Thanks for the reminder. I'm trying to figure out why the tests are not passing. If you have any idea please let me know.

joachim’s picture

Functional Javascript tests can be a bit temperamental. It can't be a problem with your MR, as api.php code is not executed.

kleiton_rodrigues’s picture

StatusFileSize
new36.73 KB

JavaScript tests can be a bit unpredictable.
The code changes in core/lib/Drupal/Core/Render/theme.api.php should not directly affect these tests, I am investigating to better understand the issue.

quietone’s picture

Category: Bug report » Task

@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"

joachim’s picture

It's a bug because if people copy this code as a sample, then their code will fail coding standard.

longwave’s picture

Status: Reviewed & tested by the community » Needs work

Let's change my_module_list's variables to match while we are here, it's much easier to read in multiline format.

joachim’s picture

+1, though 'my_module_icon' as well then too.

sonyavpaa made their first commit to this issue’s fork.

sonyavpaa’s picture

Status: Needs work » Needs review

Seperated also the my_module_list and my_module_icon arrays into multiple lines, needs review!

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Seems pretty straight forward.

  • quietone committed f79f5982 on 10.3.x
    Issue #3471544 by ronttizz, sonyavpaa, joachim, longwave, smustgrave:...

  • quietone committed db8a4ac5 on 10.4.x
    Issue #3471544 by ronttizz, sonyavpaa, joachim, longwave, smustgrave:...

  • quietone committed 9e36c151 on 11.0.x
    Issue #3471544 by ronttizz, sonyavpaa, joachim, longwave, smustgrave:...

  • quietone committed a9b0fa6d on 11.x
    Issue #3471544 by ronttizz, sonyavpaa, joachim, longwave, smustgrave:...
quietone’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: +Coding standards

@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!

Status: Fixed » Closed (fixed)

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