Style error information

https://eslint.org/docs/rules/implicit-arrow-linebreak

How to Review

## 1. Apply Patch
## 2. Review Code Changes
## 3. Confirm no Code Standard Errors
yarn & yarn lint:core-js-passing
## 4.1 If `NO` errors, mark the issue as `Reviewed & tested by the community` (Don't be shy, we're all friendly)
## 4.2 If `HAS` errors, fix it and upload a new patch (Just do it and you can!!!)

Background

- #2912962: Step 1 JS codestyle: [meta] Fix JS coding standards in core

- We adapted the airbnb coding standard (#2815077: Adopt airbnb javascript style guide v14.1 as new baseline javascript coding standards for Drupal 8 core and contrib), but we are not fully compliant to it yet.

More Information

- Using ES6 in Core
https://www.drupal.org/node/2815083

- To find JS code standard errors stats
cd core/ && yarn & yarn lint:core-js-stats

Valuable Follow-up

- N/A

CommentFileSizeAuthor
#20 2983377-20.patch1.08 KBApacheEx
#14 2983377-14.patch2.07 KBApacheEx
#12 2983377-12.patch2.07 KBApacheEx
#11 interdiff_10-11.txt310 bytesanmolgoyal74
#11 2983377-11.patch2.08 KBanmolgoyal74
#10 2983377-10.patch2.1 KBApacheEx
#7 interdiff-2983377-2-7.txt1.6 KBApacheEx
#7 2983377-7.patch2.1 KBApacheEx
#5 interdiff-2983377-2-5.txt1.59 KBApacheEx
#5 2983377-5.patch2.09 KBApacheEx
#2 2983377-2.patch1.91 KBApacheEx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drpal created an issue. See original summary.

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
Issue summary: View changes
Status: Active » Needs review
FileSize
2983377-2.patch1.91 KB

Here is a patch. I have also run yarn run build:js.

m1r1k’s picture

m1r1k CreditAttribution: m1r1k commented
Status: Needs review » Reviewed & tested by the community
GrandmaGlassesRopeMan’s picture

GrandmaGlassesRopeMan
he/him
Primary language English
Location 🌎
CreditAttribution: GrandmaGlassesRopeMan at Acquia commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/misc/form.es6.js
@@ -154,10 +154,7 @@
-      // We use id to avoid name duplicates on radio fields and filter out
-      // elements with a name but no id.

    +++ b/core/modules/block/js/block.es6.js
@@ -137,12 +137,7 @@
-            // Increment the weight before assigning it to prevent using the
-            // absolute minimum available weight. This way we always have an
-            // unused upper and lower bound, which makes manually setting the
-            // weights easier for users who prefer to do it that way.

    Can we keep these comments and just move them to the correct location?

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
Status: Needs work » Needs review
FileSize
2983377-5.patch2.09 KB
interdiff-2983377-2-5.txt1.59 KB

Yeah, that's how --fix automatically works.
I agree, comments should not be removed.
Here is updated path.

Status: Needs review » Needs work

The last submitted patch, 5: 2983377-5.patch, failed testing. View results

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
Status: Needs work » Needs review
FileSize
2983377-7.patch2.1 KB
interdiff-2983377-2-7.txt1.6 KB

A bit style tuning :)
Should be better now.

m1r1k’s picture

m1r1k CreditAttribution: m1r1k commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

This needs a reroll.

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2983377-10.patch2.1 KB

Rerolled with the latest 8.6.x

anmolgoyal74’s picture

anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commented
FileSize
2983377-11.patch2.08 KB
interdiff_10-11.txt310 bytes

Removed extra blank line.

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
FileSize
2983377-12.patch2.07 KB

rerolled with the latest changes from 8.6.x

Status: Needs review » Needs work

The last submitted patch, 12: 2983377-12.patch, failed testing. View results

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
Status: Needs work » Needs review
FileSize
2983377-14.patch2.07 KB

Magic, re-upload.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

GrandmaGlassesRopeMan’s picture

GrandmaGlassesRopeMan
he/him
Primary language English
Location 🌎
CreditAttribution: GrandmaGlassesRopeMan at Acquia commented
Status: Needs review » Reviewed & tested by the community

  • lauriii committed 29a2728 on 8.7.x 
    Issue #2983377 by ApacheEx, anmolgoyal74, drpal, m1r1k: JS codestyle:...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Version: 8.7.x-dev » 8.6.x-dev

Committed 29a2728 and pushed to 8.7.x. Thanks! ✨

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented

Hi @lauriii,

thank you, just a few notes:

1) will it be backported to 8.6.x? because you've changed the version from 8.7.x-dev to 8.6.x-dev
2) the issue still in RTBC, if no backport planned, then it should be Fixed with Version: 8.7.x-dev

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Internetdevels, Drupal Ukraine Community commented
FileSize
2983377-20.patch1.08 KB

I see one more lint issue in 8.7.x (seems new changes)
Here is patch to fix it.

  • lauriii committed 6be4122 on 8.7.x 
    Issue #2983377 follow-up by ApacheEx: JS codestyle: implicit-arrow-...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented

Good catch! Thanks @ApacheEx!

I was planning to backport this to 8.6.x but the branch is still frozen, therefore I'm still leaving this open.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Reviewed & tested by the community » Fixed

Committed e0feb5b and pushed to 8.6.x. Thanks!
Committed 5fd1248 and pushed to 8.6.x. Thanks!

I cherry-picked this back to 8.6.x as the freeze is over.

  • alexpott committed 5fd1248 on 8.6.x authored by lauriii 
    Issue #2983377 follow-up by ApacheEx: JS codestyle: implicit-arrow-...

  • alexpott committed e0feb5b on 8.6.x authored by lauriii 
    Issue #2983377 by ApacheEx, anmolgoyal74, drpal, m1r1k: JS codestyle:...

Status: Fixed » Closed (fixed)

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