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

Add stylelint to builds and watch #1885

Merged
merged 4 commits into from
Jun 11, 2017
Merged

Conversation

samrap
Copy link

@samrap samrap commented Apr 24, 2017

Stylelint is an awesome tool that brings the many benefits of linting to CSS and Sass. In Sage, we already use Eslint for JavaScript and PHPCS for the back end. I find that often times, our Sass files are left with no coding standards. Poorly written Sass can be very difficult to understand and can leave you with unforeseen bugs and duplicate code.

This PR adds Stylelint to the build and watch processes via the stylelint-webpack-plugin and extends Stylelint's recommended config as a base. To be friendly for CI builds, Stylelint will fail on error when running a build, but will only emit errors while watching, ensuring that it does not break the watch task.

Adding Stylelint is a no-brainer. However, the implementation could use some discussion. The easiest way is with the stylelint-webpack-plugin which seems to have a smaller community and moderate support at best. I am willing to discuss other options, but have been used the plugin on development of 4 highly custom sites across my team with no issues. It has really helped clean up our Sass code style as a whole!

@QWp6t
Copy link
Member

QWp6t commented May 7, 2017

Looks like we're on board with this. Just need to review some of the stuff it enforces. 👍

@samrap
Copy link
Author

samrap commented May 8, 2017

Sounds good. Stylelint is very similar to eslint in that you can override rules straight in the .stylelintrc. Let me know if you would like any help getting this merged!

@retlehs
Copy link
Member

retlehs commented Jun 11, 2017

thanks a lot for this PR! and thanks for doing that push access to your branch thing (i made one minor update w/ the comments in sass files and fixed a conflict on the yarn lockfile)

@retlehs retlehs merged commit 556f08a into roots:master Jun 11, 2017
@samrap
Copy link
Author

samrap commented Jun 12, 2017

Happy to help! Thanks for all your hard work on Roots!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants