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: Add lint-style script based on stylelint #12722

Merged
merged 4 commits into from
Dec 19, 2018

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Dec 9, 2018

Description

This PR adds lint-style to wp-scripts exposed by @wordpress/scripts package.

It helps enforce coding style guidelines for your style files. It uses stylelint with the set of default rules provided. You can override them with your own rules as described in stylelint user guide.

How has this been tested?

npm run lint-css still works properly in the repository.
./node_modules/.bin/wp-scripts lint-style '**/*.scss' also works properly

@gziolo gziolo added [Type] Build Tooling Issues or PRs related to build tooling [Tool] WP Scripts /packages/scripts labels Dec 9, 2018
@gziolo gziolo added this to the 4.8 milestone Dec 9, 2018
@gziolo gziolo requested review from ntwb and a team December 9, 2018 17:07
@gziolo gziolo self-assigned this Dec 9, 2018
@adamsilverstein
Copy link
Member

Looks good to me!

packages/scripts/README.md Outdated Show resolved Hide resolved
@ntwb
Copy link
Member

ntwb commented Dec 9, 2018

I think we should follow up in a subsequent pull request to add docs and a configuration for linting SCSS files.


I originally wrote the above and what I was going to write here was about using lint-style as the command instead of lint-css, expanding on this thinking, we would want to add lint-scss for the scss config and then have lint-style run both lint-css and lint-scss?

@ntwb
Copy link
Member

ntwb commented Dec 9, 2018

The .stylelintrc.json → packages/scripts/config/.stylelintrc.json change copies the Gutenberg specific .stylelintrc.json stylelint configuration to be distributed by @wordpress/scripts

This file should remain in its current location and a new file should be created at packages/scripts/config/.stylelintrc.json that contains:

{
	"extends": "stylelint-config-wordpress"
}

The README.md should be updated to:

It uses [stylelint](https://github.com/stylelint/stylelint) with the [stylelint-config-wordpress](https://github.com/WordPress-Coding-Standards/stylelint-config-wordpress) configuration per the [WordPress CSS Coding Standards](https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/).

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.

The notes at #12722 (comment) should be addressed before merge

@gziolo
Copy link
Member Author

gziolo commented Dec 10, 2018

Thanks @ntwb for sharing your feedback. I'm in transit and will get back to it later this week. Unless you want to apply proposed changes yourself :)

@gziolo
Copy link
Member Author

gziolo commented Dec 12, 2018

I originally wrote the above and what I was going to write here was about using lint-style as the command instead of lint-css, expanding on this thinking, we would want to add lint-scss for the scss config and then have lint-style run both lint-css and lint-scss?

What would be the difference? I assumed they are the same in the context of Gutenberg.

@gziolo gziolo force-pushed the update/move-stylelint-scripts branch from 8cb82a6 to 668eebb Compare December 12, 2018 09:41
@gziolo gziolo requested a review from ntwb December 12, 2018 09:45
@gziolo
Copy link
Member Author

gziolo commented Dec 12, 2018

@ntwb I applied all your suggestions in 668eebb.

@gziolo gziolo force-pushed the update/move-stylelint-scripts branch 2 times, most recently from 8262733 to bee953b Compare December 13, 2018 16:35
@gziolo gziolo force-pushed the update/move-stylelint-scripts branch from bee953b to 8897178 Compare December 19, 2018 10:46
@gziolo gziolo merged commit 47950c3 into master Dec 19, 2018
@gziolo gziolo deleted the update/move-stylelint-scripts branch December 19, 2018 11:40
youknowriad pushed a commit that referenced this pull request Jan 9, 2019
* Scripts: Add lint-css script based on stylelint

* Update packages/scripts/README.md

Co-Authored-By: gziolo <[email protected]>

* Update default config for lint-style script

* Scripts: Extend description for the package
youknowriad pushed a commit that referenced this pull request Jan 9, 2019
* Scripts: Add lint-css script based on stylelint

* Update packages/scripts/README.md

Co-Authored-By: gziolo <[email protected]>

* Update default config for lint-style script

* Scripts: Extend description for the package
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Tool] WP Scripts /packages/scripts [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants