Problem/Motivation

eslint is not run on files during commit checks: see test run logs for #3309318: Remove IE11 workaround from CKEditor 5 eslint is not listed.

Problems comes from a bad reroll of #3278415: Remove usages of the JavaScript ES6 build step, the build step itself, and associated dev dependencies where a line in the commit check script was removed.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 core-fix-eslint-commit-check-3309807-7.patch1.11 KBnod_
core-fix-eslint-commit-check.patch619 bytesnod_

Comments

nod_ created an issue. See original summary.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Related issues: +#3309800: /core/modules/ckeditor5/js/ckeditor5.js fails JS linting, +#3278415: Remove usages of the JavaScript ES6 build step, the build step itself, and associated dev dependencies
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Excellent find! 👍

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented 
+++ b/core/scripts/dev/commit-code-check.sh
@@ -384,6 +384,7 @@
     if [[ -f ".eslintrc.passing.json" ]]; then

Wonder if we should remove this check, why wouldn't the file exist?

nod_’s picture

nod_
Primary language French
Location Lille
commented

I guess if you're in the wrong directory it wouldn't exist :p

That's fair enough, no strong feeling either way.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Yeah, I was trying to get at the fact the script would have likely failed if that check were not present, so changing it should prevent this from happening again.

nod_’s picture

nod_
Primary language French
Location Lille
commented
StatusFileSize
new core-fix-eslint-commit-check-3309807-7.patch1.11 KB

seems to work as intended: 

Oops! Something went wrong! :(

ESLint: 8.23.0

Failed to read JSON file at drupal/core/.eslintrc.passing.json:

Cannot read config file: drupal/core/.eslintrc.passing.json
Error: ENOENT: no such file or directory, open 'drupal/core/.eslintrc.passing.json'


eslint: failed

Made that piece of code similar to the yaml linting, changed the name of the variable, and added a printf to say the check passed as expected.

  • lauriii committed b5d5cdd on 10.1.x 
    Issue #3309807 by nod_: Fix commit eslint checks following removal of...

  • lauriii committed 8221a5f on 10.0.x 
    Issue #3309807 by nod_: Fix commit eslint checks following removal of...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Wonder if we should remove this check, why wouldn't the file exist?

I think that's from a time when we had supported branches that didn't ship with the eslint configuration.

Confirmed manually on local that the fix works.

Committed b5d5cdd and pushed to 10.1.x. Also cherry-picked to 10.0.x. Thanks!

Status: Fixed » Closed (fixed)

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