Skip to content

enhancement: add .prettierignore file#7187

Merged
sokra merged 3 commits intowebpack:masterfrom
byzyk:enhancement/prettierignore
May 25, 2018
Merged

enhancement: add .prettierignore file#7187
sokra merged 3 commits intowebpack:masterfrom
byzyk:enhancement/prettierignore

Conversation

@byzyk
Copy link
Copy Markdown
Contributor

@byzyk byzyk commented May 3, 2018

What kind of change does this PR introduce?

enhancement

Did you add tests for your changes?

no

If relevant, link to documentation update:

n/a

Summary

Adds .prettierignore to prevent IDEs plugins from autoformatting wrong files (eg. JSON files or some test related files).

Does this PR introduce a breaking change?

no

Other information

Tested in Atom (prettier-atom plugin). Feedback from other IDEs is highly appreciated.

@webpack-bot
Copy link
Copy Markdown
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

Comment thread package.json
"code-lint": "eslint setup lib bin hot buildin \"test/*.js\" \"test/**/webpack.config.js\" \"examples/**/webpack.config.js\" \"schemas/**/*.js\"",
"type-lint": "tsc --pretty",
"fix": "yarn code-lint --fix",
"pretty": "prettier \"setup/**/*.js\" \"lib/**/*.js\" \"bin/*.js\" \"hot/*.js\" \"buildin/*.js\" \"test/*.js\" \"test/**/webpack.config.js\" \"examples/**/webpack.config.js\" \"schemas/**/*.js\" \"declarations.d.ts\" --write",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You Want to keep the npm script

Copy link
Copy Markdown
Contributor Author

@byzyk byzyk May 14, 2018

Choose a reason for hiding this comment

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

I guess it's redundant to the one above it (yarn fix), isn't it? In addition to that, it's not being used anywhere else at all.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is not redundant, one is ESLint other is Prettier

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As per @sokra explanation in #7276 we're using prettier plugin for ESLint, meaning ESLint runs prettier as a rule and reports errors/warnings. Also, ESLint will format code based on prettier rules when --fix flag is used, meaning does exactly the same as pretty script does.

Taking into consideration the above and the fact that pretty script remains unused I thought that we could get rid of it unless someone is using yarn pretty explicitly, but I doubt that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I use yarn pretty explicitly. It's faster than running eslint and doesn't do non-prettier changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My bad. Didn't know it's being used by people. I've put it back, just moved the --write flag to be more aligned with official docs.

@webpack-bot
Copy link
Copy Markdown
Contributor

webpack-bot commented May 15, 2018

For maintainers only:

  • This need to be documented (issue in webpack/webpack.js.org will be filed when merged)

@sokra sokra merged commit b77addd into webpack:master May 25, 2018
@sokra
Copy link
Copy Markdown
Member

sokra commented May 25, 2018

Thanks

@byzyk byzyk deleted the enhancement/prettierignore branch June 4, 2018 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants