CommentFileSizeAuthor
#5 2975105-5.patch1.63 KBGrandmaGlassesRopeMan
#2 2975105-2.patch1.71 KBjustafish

Comments

justafish created an issue. See original summary.

justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented
Status: Active » Needs review
StatusFileSize
new 2975105-2.patch1.71 KB
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

I am trying to figure out why this got introduced in the first place. For example #2809343: Add ESLint to package.json has it in there.

@matt: introduce a syntax violation in some code, we need the exit 0 otherwise it's ugly.

It would be nice to understand why it got added in the first place.

On the other hand, I totally think that these command should exit with 1, if they fail.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2975105-2.patch, failed testing. View results

GrandmaGlassesRopeMan’s picture

GrandmaGlassesRopeMan
he/him
Primary language English
Location 🌎
commented
Status: Needs work » Needs review
Issue tags: +JavaScript
StatusFileSize
new 2975105-5.patch1.63 KB
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

The only people this might could cause issues are core developers, because noone else is using these scripts. It is not touching anyone with a running Drupal site.

alexpott’s picture

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

Committed a99a141 and pushed to 8.6.x. Thanks!

Patch didn't apply to 8.5.x.

  • alexpott committed a99a141 on 8.6.x 
    Issue #2975105 by justafish, drpal, dawehner: JavaScript tests shouldn't...

Status: Fixed » Closed (fixed)

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