-
Notifications
You must be signed in to change notification settings - Fork 6
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
FLUID-6555: add initial configuration for CSS and Sass linting #20
Conversation
Based on our experience with Justin's recent pull, I think a good litmus test of this would be to create a "companion pull" against Infusion that uses the work in progress here for its linting. That would allow us to confirm whether there are linting errors in Infusion itself and to talk about either adjusting rules or fixing errors. |
@the-t-in-rtf Yes, that makes sense. In FLUID-6551 I'd proposed exploring alternatives to Stylus; this PR currently doesn't lint Stylus files (there's an experimental Stylus linting extension for Stylelint but I haven't tested it) so it may not have any impact on Infusion. |
At least until the UI components are moved out of the repo, there are still raw CSS files that should be included in linting. See my comment on FLUID-6551. It would also be nice to have support for Stylus, in looking at your list on FLUID-6551, I realised how many projects would benefit from having that. How much work are we talking? |
@the-t-in-rtf I'm going to try updating this PR with some Stylus code and the stylus extension and see if it works. I'll also open a PR against infusion once I've done that. |
@the-t-in-rtf So, I've tested a bit, and it's not going to work to have a shared config for CSS, SCSS and Stylus. We can have a shared CSS/SCSS configuration as the syntaxes are more compatible but we would need a separate configuration to lint Stylus. |
@the-t-in-rtf I'm going to explore the use of stylint (not to be confused with stylelint) for independently linting Stylus files. (Props to @BlueSlug for suggesting I look for other linters that could handle Stylus.) |
Glad to hear it. I'll wait to hear from you. |
@the-t-in-rtf After further investigating, I don't believe that stylint is a viable option. It hasn't been updated in over a year and has a number of reported issues; most significantly the There is an "official" linter: https://github.com/stylus/stlint However, it doesn't have any existing Grunt integrations and if the ultimate goal is what you've outlined in #21, I'm not sure it makes sense to spend time developing a Grunt integration for stlint. Thoughts? |
Thank you for putting in the time to research this, @greatislander. Adding at least some CSS coverage seems better than continuing with none. I will take a look at the other pull shortly. |
I think the associated Infusion pull is looking good, I'll check in with @amb26 and see if we can round out this pair of pulls soon. If the work on the standard config is likely to be available, I think it would make sense to:
|
@the-t-in-rtf Steps 1-3 are now complete (see: fluid-project/stylelint-config-fluid). |
The infusion pull still uses a hard-coded |
@the-t-in-rtf Thanks for the reminder— I've updated that PR. It's still referencing this branch for |
* greatislander/FLUID-6555: FLUID-6555: remove empty blocks and one extraneous comment FLUID-6555: remove extraneous comment from Reorderer.css FLUID-6555: remove unnecessary comment break FLUID-6555: reposition comment in OverviewPanel.css FLUID-6555: update fluid-lint-all to 1.0.4 FLUID-6555: update fluid-lint-all, cover all CSS and SCSS files FLUID-6555: update fluid-lint-all, adjust .fluidlintrc.json FLUID-6555: fix SCSS error FLUID-6555: apply linting rules to Sass FLUID-6555: update to fluid-grunt-lint-all 2.0.0 FLUID-6555: use central config FLUID-6555: lint more files FLUID-6555: lint more files, apply bracket spacing rules FLUID-6555: address CSS linting issues from fluid-project/fluid-grunt-lint-all#20
This is an initial attempt at FLUID-6555 which uses the Stylelint configuration from inclusive-design/idrc. The configuration rules should be able to be overridden as documented in #17. Hoping to start the discussion from here, including an evaluation of these rules to determine if they are suitable as a starting point for Fluid Project… projects!