Setup Scss Lint
marcobiedermann opened this issue · 24 comments
Add Scss lint task to Gulpfile.js via scss-lint
Will definitely look into it and add a task to Grunt.
I would favor something more like Stylelint since it doesn't have the Ruby dependency. Instead it runs via JS using post-css.
I have no preference as I don't use either of them. @marcobiedermann ?
@nathanjessen Yeah, Stylelint looks promising, I have not used it at all but will take a look at it soon. I absolutely agree with you, having as few dependencies as possible is a good approach. So let's remove Sass-Lint and use Stylelint instead.
Hi, has there been progress on this? Mind if I help?
@tricinel not yet. Sure, go ahead :)
Okidoki :)
@matejlatin, right, so I have this done now, see https://github.com/tricinel/Gutenberg/blob/feature/scss-lint/Gruntfile.js#L121. I haven't merged yet because I didn't know if you had a preference for the linter config. I've used Github's primer (https://github.com/primer/stylelint-config-primer), which we can extend however we want. The project's code passes nevertheless, but it has some warnings (no errors).
Let me know if you have a preference for the scss linting options and I'll merge my feature branch into master and submit a pull request.
Also, I read above you wanted to avoid Ruby etc, but grunt-contrib-sass is using it nevertheless. I'm of the same mind: avoiding unnecessary dependencies. Would it be ok with you if I spent just a bit of time doing the following:
- replace grunt with gulp
- no longer have globally installed dependencies (i.e. npm install -g )
- only required system-wide dependency should be node (and npm of course)
- create npm scripts instead, so you can run npm start (which would trigger something like your grunt default)
- with linting in place, we can create build tasks that will allow us to use Travis.ci as well (I saw an issue at some point). This will basically mean that we integrate linting, scss->css, minify into one task and if it succeeds, the build passes.
Loving the project!!! Let me know what you think.
What about, instead of using wrappers—like Gulp, Grunt and everything else—just use something like Jake just to organize the standard CLI tools for Sass, linting, minification, etc.
Here is an example I wrote for JSDocs: https://github.com/jsdocs/jsdocs/blob/frontend/Jakefile
The file can the optimized to avoid the repetition, but I still think is simpler and more transparent. What do you think?
That could work as well. I have a preference for gulp/grunt just because it's more widely known and used I guess, but they all do the same thing: put together cli tools. I like gulp because of its streams and how you can pipe things together.
On the same note, we can just use npm scripts for everything, and have npm run lint, which would just be a shortcut for the stylelint tool.
Up to you guys really. I haven't worked with Jake before, so if we go down that route, perhaps @jameskolce would be more entitled to write the implementation. It doesn't look complex.
Yeah, so Jake is on the same line as a tool like Make but with JS. Is like the middle point between gulp/grunt and plain npm scripts. We wouldn't need the gulp/grunt cli wrappers because we would use the original cli tools directly, and we don't need to have a mess in the package.json file either.
Also, we can still use pipes, the operating system already implements them with "|" if we need to use them, though it might not be necessary.
Jake can still be used with npm scripts, that is usually the good way to do it. We can just call Jake commands from npm.
I can work on that if we all agree on it!
Yep, I agree. I will push that pull request that just handles the stylelint and then we can come to an agreement on the build process. :) Not that what we currently have doesn't work, btw! Just to make it better.
Yeah, perfect. I agree with that, and if @matejlatin agrees too then we can do it. I also think that the current flow works but in this way it would be easier and simpler.
Guys, to be completely honest I have no idea what you're discussing for most of these. We'd need to have a proper chat but it seems like a lot of changes. I was planning to work on v2 some time next year maybe that would be a good time to introduce these major changes?
One question I have is why remove Grunt? I quite like the fact that it runs on Grunt... is it simply because not everyone can use it (if they don't use Grunt)?
@tricinel I see you're suggesting of replacing Grunt with Gulp... any particular reason for that?
@matejlatin Just preference. Nothing else. I started out with grunt and after a while switched everything to gulp. But I love both equally, with a slight preference to gulp streams. I think, if you're tired of writing configuration and want instead to write the code that connects micro-tasks together, then gulp is your tool.
v2 sounds like a good milestone to incorporate any potential changes to the flow, if you ask me. I think both myself and @jameskolce are here to help if you decide to make those changes.
@tricinel @jameskolce I merged the pull request. But shouldn't we be linting all the scss files? Currently we only lint the config and the main file.
How do we go about adding Travis CI now? I think there's already an issue for that...
Hmmm...I might have messed up the grunt config if it's not doing all the scss files in the style directory. I'll take a look.
Travis CI, can do that as well, but only partly - you'd need to sign up with your github account for a free travis account and add this repo there as part of your account. I can take care of the config + npm scripts for testing. This would mean that every time someone pushes (or makes a pull request), the command npm test runs, and npm test would just be, probably, grunt lint.
I'll let you know about the scss files paths.
Cool, should we add the lint task to the default watch task as well?
@tricinel changed src: ['*.scss'] to src: ['**/*.scss'] and it goes through all the scss files. Will do some more cleaning up now...
What rules sorting are we using? So I can set up my plugin and save some time on cleaning up...
Hi @matejlatin, it's here: https://github.com/primer/stylelint-config-primer, all rules are properly explained in their readme. And we can just override any rules in .stylelintrc by adding a "rules" property.
And yes, we should add it to watch. Sorry for the late replies, I was traveling :)
Hey @tricinel what I meant was properties sorting. Lots of warnings are "expected property x before property y" and if my Sublime plugin sorting matched the one we're using it would save me a lot of time cleaning up.
No worries, mate ;)
Oh, I see, I'm not sure about that. Usually, I just do alpha sort. But looking at the property in primer, it seems it's this: https://github.com/primer/stylelint-config-primer/blob/1ac303e9a633ee38d168a014dcea10eaf5a95aab/index.js#L45-L215 but we can just change it to alphabetical: http://stylelint.io/user-guide/rules/declaration-block-properties-order/
Ah ok that's what I was looking for :) I should be able to set up my plugin now...