phetsims / perennial

Maintenance tools that won't change with different versions of chipper checked out
MIT License
2 stars 5 forks source link

`grunt lint` in perennial seems to be missing errors #244

Closed jonathanolson closed 3 years ago

jonathanolson commented 3 years ago

If I add a lint error to perennial and run grunt lint, the following happens:

$ grunt lint
Running "lint" task
Running "lint" task

Done.

Done.

Whereas grunt lint-everything:

$ grunt lint-everything
Running "lint-everything" task

C:\Users\olson\phet\git\perennial\js\common\isGitRemoteDifferent.js
  31:1   error  'artasdrtart' is not defined  no-undef
  31:12  error  Missing semicolon             semi

✖ 2 problems (2 errors, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

Warning: 2 errors and 0 warnings Use --force to continue.

Aborted due to warnings.

However pre-commit hooks and IDEs seem to be properly linting.

@zepumph any thoughts on this?

zepumph commented 3 years ago

My first question is if --eslint-disable-cache changes anything for you.

zepumph commented 3 years ago

With this patch:


Index: js/common/isGitRemoteDifferent.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/isGitRemoteDifferent.js b/js/common/isGitRemoteDifferent.js
--- a/js/common/isGitRemoteDifferent.js (revision 7ebacc61adfaaab7724a8a4a70b4ed3f9a97dcf2)
+++ b/js/common/isGitRemoteDifferent.js (date 1631550444119)
@@ -20,7 +20,7 @@
  * @returns {Promise.<boolean>}
  */
 module.exports = async function( repo ) {
-  assert( typeof repo === 'string' );
+  assert( typeof repo === "string" );

   const branch = await getBranch( repo );
   const currentSHA = await gitRevParse( repo, 'HEAD' );

the cache didn't change anything

 Michael ~/PhET/git/perennial (master)
 $ grunt lint
Running "lint" task
Running "lint" task

Done.

Done.

 Michael ~/PhET/git/perennial (master)
 $ grunt lint --disable-eslint-cache
Running "lint" task
Running "lint" task

Done.

Done.
zepumph commented 3 years ago

When I log the patterns passed to lint, it shows that it is linting chipper:

in lint.js:


  console.log( patterns, options );
 Michael ~/PhET/git/perennial (master)
 $ grunt lint --disable-eslint-cache
Running "lint" task
Running "lint" task
[ '../chipper' ] { cache: false, format: undefined, fix: undefined, warn: true }

Done.

Done.
zepumph commented 3 years ago

Looks like this broke during https://github.com/phetsims/perennial/commit/1647619ce05cc306e306166c8c5964b35a949cef, because we got rid of the repo option, but perennial's pattern wasn't specified directly. Fix coming in hot!

zepumph commented 3 years ago

A couple of fixes:

To test:

grunt lint -> lints perennial grunt lint --patterns=../perennial -> lints perennial grunt lint --patterns=../perennial,../chipper -> lints both repos grunt lint --patterns=../chipper -> lints just chipper, not perennial

The only thing we COULD do, and I had implemented for a second before deleting, is to manually add perennial when the patterns option is provided by the user, but this seemed too clever, and not that helpful. @samreid, since you worked on https://github.com/phetsims/chipper/issues/1001, will you please review.

samreid commented 3 years ago

I reviewed the commits, and tested grunt lint in perennial with a bad lint. Everything seems nice, thanks! Closing.