search

phetsims / perennial

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

Maintenance.updateCheckouts() doesn't lint when building sims #344

Open zepumph opened 8 months ago

zepumph commented 8 months ago

This was surprising to me, since when you go to deploy release branches, it will tsc and lint. I found it really helpful over in https://github.com/phetsims/phet-io/issues/1974 that there was type checking when building. Running updateCheckouts after updateDependencies ended up saving me on 3 type errors I would have had to handle in a less opportune time. So let's also lint too! I'm going to set the default to updateCheckouts to be to always lint.

zepumph commented 8 months ago

After the above commit, I catch my lint error I got when deploying RCs now in updateCheckouts():


C:\Users\mjkauzmann\PHET\git\release-branches\gravity-and-orbits-1.6\phet-io-wrappers\common\js\PhetioClient.ts
  633:109  warning  Unused eslint-disable directive (no problems were reported from '@typescript-eslint/no-floating-promises')

✖ 1 problem (0 errors, 1 warning)
  0 errors and 1 warning potentially fixable with the `--fix` option.

@jonathanolson please spot check and let me know if you prefer a different default.

jonathanolson commented 8 months ago

Is the full update checkouts clean (with linting)? Linting doesn't seem like something I've really supported once things are a release branch, as it has caused major issues based on npm in the past. However as long as we keep our actual deploy/build processes for release branches without lint, I think it's fine (and very beneficial to lint updateCheckouts(), if everything is clean right now).

zepumph commented 8 months ago

Hmm, well I ran into this issue BECAUSE deployReleaseCandidates() failed on a lint. So . . . I'm not sure what you recommend.