Closed Hirse closed 4 years ago
When I run npm run build
I get this message:
Running "browserify-components" task
>> ./components/.eslintrc.json/.eslintrc.json.js does not exist. If this component is obsolete, please remove that directory or perform a clean build.
You can also see it in the CI builds.
This is because we use readdir
and assume that inside the components
folder are only other folders and no files. So we need to change the logic a little bit.
Before we did run jshint
as part of build
now we have to manually run npm run lint
. So we should either:
npm run lint && grunt
on build ornpm run lint
must be run manually in the CONTRIBUTING.mdAlso we now have two options to fix formatting errors:
npm run format
npm run lint -- --fix
Do we need both?
If yes should be change format
to run eslint . bin/* --fix
instead? Should we rename it to e.g. lint-fix
?
This will be the output when there are linting errors:
PS P:\ungit> npm run lint
> ungit@1.5.7 lint P:\ungit
> eslint . bin/*
P:\ungit\Gruntfile.js
350:5 error Insert `;` prettier/prettier
✖ 1 problem (1 error, 0 warnings)
1 error and 0 warnings potentially fixable with the `--fix` option.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! ungit@1.5.7 lint: `eslint . bin/*`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the ungit@1.5.7 lint script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
@campersau thanks for the detailed review.
components
directory.CONTRIBUTING.md
npm run format
to run npm run lint -- --fix
I had removed the linting from the build step to speed up build time and to not stop the build when there are formatting issues. The linting is only really required before commit. During development linting or formatting issues should not stop the build.
We could consider adding the linting as pre-commit hook or adding it to the build in a non-blocking way (i.e. only issue warnings).
Configure linting rules specific to folder using
.eslintrc.json
filesclicktest
: mochacomponents
: browserpublic/source
: browsersource
: nodetest
: mochaUsing files in the folders allows editor plugins to get the correct config when a file is opened
npm run lint
scriptUse recommended configuration (of ESLint and the plugins) with the following exceptions
no-unused-vars
: Allow unused parameter as that seems to be current code styleno-process-exit
: Currently used intentionallyno-control-regex
: We have to use control characters as they appear in messages from gitno-prototype-builtins
: Only one occurrence that we can refactor latermocha/no-mocha-arrows
: Mocha discourages the use of arrow functions, which are used in the code basemocha/no-setup-in-describe
: Can be moved tobeforeAll
blocks latermocha/no-identical-title
: Consider whether current naming is optimalThe current configuration is aimed at providing the most value without requiring big refactorings for now. Future rules to consider might be:
no-var
: Requireconst
orlet