Skip to content

Commit

Permalink
remove format option to eslint, #1427
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Kauzmann <michael.kauzmann@colorado.edu>
  • Loading branch information
zepumph committed Mar 22, 2024
1 parent 1791f8f commit 5149bb1
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 185 deletions.
12 changes: 2 additions & 10 deletions eslint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,8 @@ We use these tools at several stages of development:
## Usage

For developers looking to ensure their changes pass ESLint, the typical entry point is to run `grunt lint`.
See `grunt lint --help` for options. If your code passes
`grunt lint`, then it is good to merge. You can add the `--format` option to achieve formatting that comes close to
PhET's code standards defined by the Webstorm code style
[here](https://github.com/phetsims/phet-info/blob/main/ide/idea/phet-idea-codestyle.xml). That is considered the ground
truth for formatting currently, and `--format` attempts to get as close as we can to those rules.
See `grunt lint --help` for options. If your code passes `grunt lint` (and other git hooks), then it is good to
merge.

## Shared Configuration Files

Expand All @@ -39,15 +36,10 @@ Here is a list of all the available configuration files and why to use them for
- `node_eslintrc.js` expands on the base rules and adds configuration only intended for Node.js code (i.e. `perennial`).
- `sim_eslintrc.js` expands on the base rules and adds configuration intended for code run in sims (think of this as es5
sim rules)
- `sim_eslintrc.js'` expands on the sim rules and adds configuration for sims that have no es5 in them (
i.e. `wave-interference`)
- `format_eslintrc.js` contains additional rules used for enforcing code formatting. These are not required, they are
just recommended.

So here is the hierarchy of chipper's config files. Indentation symbolized the "extends" relationship.

```
format_eslintrc.js
.eslintrc.js
node_eslintrc.js
sim_eslintrc.js
Expand Down
153 changes: 0 additions & 153 deletions eslint/format_eslintrc.js

This file was deleted.

5 changes: 0 additions & 5 deletions js/grunt/Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,6 @@ Minify-specific options:
`lint js files. Options:
--disable-eslint-cache: cache will not be read or written
--fix: autofixable changes will be written to disk
--format: Append an additional set of rules for formatting
--chip-away: output a list of responsible devs for each repo with lint problems
--disable-with-comment: add an es-lint disable with comment to lint errors
--repos: comma separated list of repos to lint in addition to the repo from running`,
Expand All @@ -355,7 +354,6 @@ Minify-specific options:
// --disable-eslint-cache disables the cache, useful for developing rules
const cache = !grunt.option( 'disable-eslint-cache' );
const fix = grunt.option( 'fix' );
const format = grunt.option( 'format' );
const chipAway = grunt.option( 'chip-away' );
const disableWithComment = grunt.option( 'disable-with-comment' );

Expand All @@ -364,7 +362,6 @@ Minify-specific options:
const lintReturnValue = await lint( [ repo, ...extraRepos ], {
cache: cache,
fix: fix,
format: format,
chipAway: chipAway,
disableWithComment: disableWithComment
} );
Expand All @@ -380,7 +377,6 @@ Minify-specific options:
// --disable-eslint-cache disables the cache, useful for developing rules
const cache = !grunt.option( 'disable-eslint-cache' );
const fix = grunt.option( 'fix' );
const format = grunt.option( 'format' );
const chipAway = grunt.option( 'chip-away' );
const disableWithComment = grunt.option( 'disable-with-comment' );
assert && assert( !grunt.option( 'patterns' ), 'patterns not support for lint-all' );
Expand All @@ -392,7 +388,6 @@ Minify-specific options:
const lintReturnValue = await lint( getPhetLibs( repo, brands ), {
cache: cache,
fix: fix,
format: format,
chipAway: chipAway,
disableWithComment: disableWithComment
} );
Expand Down
21 changes: 4 additions & 17 deletions js/grunt/lint.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ const lintOneRepo = async ( repo, options ) => {

options = _.assignIn( {
cache: true,
fix: false,
format: false
fix: false
}, options );

// For PhET's custom cache, see CacheLayer
Expand All @@ -71,7 +70,7 @@ const lintOneRepo = async ( repo, options ) => {

const hash = crypto.createHash( 'md5' ).update( tsconfigFile + packageJSON ).digest( 'hex' );

const eslintConfig = {
const eslint = new ESLint( {

// optional auto-fix
fix: options.fix,
Expand All @@ -96,18 +95,7 @@ const lintOneRepo = async ( repo, options ) => {

// If no lintable files are found, it is not an error
errorOnUnmatchedPattern: false
};

const config = {};
const configExtends = [];
if ( options.format ) {
configExtends.push( '../chipper/eslint/format_eslintrc.js' );
}

config.extends = configExtends;
eslintConfig.baseConfig = config;

const eslint = new ESLint( eslintConfig );
} );

const results = await eslint.lintFiles( repoToPattern( repo ) );

Expand Down Expand Up @@ -148,7 +136,6 @@ const lint = async ( originalRepos, options ) => {

options = _.merge( {
cache: true,
format: false, // append an extra set of rules for formatting code. TODO: remove this option, these are inside the main rules now, https://github.com/phetsims/chipper/issues/1415
fix: false, // whether fixes should be written to disk
chipAway: false, // returns responsible dev info for easier chipping.
disableWithComment: false, // replaces failing typescript lines with eslint disable and related comment
Expand Down Expand Up @@ -198,7 +185,7 @@ const lint = async ( originalRepos, options ) => {
} );
worker.postMessage( {
repos: batchOfRepos,
options: _.pick( options, [ 'cache', 'format', 'fix' ] )
options: _.pick( options, [ 'cache', 'fix' ] )
} );
} );
};
Expand Down

0 comments on commit 5149bb1

Please sign in to comment.