Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scripts: Ignore linting files located in build folders by default #15977

Merged
merged 2 commits into from
Jun 5, 2019

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Jun 4, 2019

Description

Closes #15407.

It should give very good defaults with the related proposal #15890 which provides defaults for file pattern matching.

Describe the bug

Running the JS linter will lint the build/ folder, resulting in hundreds of errors because it's minified, etc.
When using wp-scripts to build blocks or other plugins that use wp.element, wp.component, etc. For example.>
Expected behavior

The build/ folder should be ignored by the linter config.

This PR ensures that by default all files located in build and node_modules folders are ignored when using wp-scripts for all 3 types of linter:

  • lint-js
  • lint-pkg-json
  • lint-style

How has this been tested?

This is quite difficult to test without publishing to npm. I personally tested it with Gutenberg repository by playing with configuration files. This is how you can run commands:

  • npx wp-scripts lint-js .
  • npx wp-scripts lint-pkg-json .
  • npx wp-scripts lint-style '**/*.scss'

I deleted .eslintignore from the root folder of Gutenberg and played with defaults:

  • packages/scripts/config/.eslintignore
  • packages/scripts/config/.npmpackagejsonlintignore
  • packages/scripts/config/.stylelintignore

I had to add some linting errors in random files to ensure it all works.

Then I reverted changes in those config files and copied them to the root folder. I tried again to modify those config files in the root folder running all commands again.

I also ensured that when I add violations to files inside build or node_modules nested subfolder, those errors are ignored.

@gziolo gziolo added [Type] Bug An existing feature does not function as intended [Package] Scripts /packages/scripts labels Jun 4, 2019
@gziolo gziolo requested review from iandunn and swissspidy June 4, 2019 09:11
@gziolo gziolo self-assigned this Jun 4, 2019
Copy link
Member

@ntwb ntwb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gziolo gziolo force-pushed the update/wp-scripts-ignored-files branch from 66fc004 to f62daad Compare June 5, 2019 08:09
@gziolo gziolo added this to the 5.9 (Gutenberg) milestone Jun 5, 2019
@gziolo gziolo merged commit c85e28f into master Jun 5, 2019
@gziolo gziolo deleted the update/wp-scripts-ignored-files branch June 5, 2019 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Scripts /packages/scripts [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lint-js task lints build/ directory
2 participants