Skip to content
This repository has been archived by the owner on Jan 21, 2022. It is now read-only.

Recommend Stylelint to lint Sass files #202

Merged
merged 1 commit into from
Oct 8, 2019

Conversation

barrymcgee
Copy link
Contributor

Historically, we've used scss-lint and latterly sass-lint to lint Sass files. These have now both fell by the wayside to be superseded by Stylelint which is built with React, Prettier et al in mind.

We already have Prettier, why do we need this? Prettier is only concerned with style, Stylelint is concerned with correctness and will flag issues which could cause bugs and code smells such as duplicate rules in the same scope, excessive nesting etc.

Thoughts?

@bartaz
Copy link
Contributor

bartaz commented Sep 2, 2019

With Vanilla being our main source of the styles I think we should use Vanilla style linting configuration by default, so that individual project styles are consistent with Vanilla.
So I think it should be Vanilla to adopt any new linting tools first.

I don't mind switching to a new one. The question is - does stylelint provide any benefits over our current style linting?

@nottrobin
Copy link
Contributor

Yeah, as @matthewpaulthomas pointed out at some point in response to something I suggested - practices, per the name, should probably be things we actually do in practice.

I'm very happy to adopt stylelint, but I think we should probably adopt it first and then define it as a practice once we've actually implemented it in a few places.

Maybe this PR could just remain open until then? Not sure what the best process is for tracking new ideas that aren't quite solid enough yet to be practices.

@anthonydillon
Copy link
Contributor

Sine the latest from the scss-lint project doesn't sound promising I think we need to find a replacement and Stylelint looks like a good option. @barrymcgee would you mind replacing scss-lint in a project such as jaas.ai to iron out issues and report back here?

@barrymcgee
Copy link
Contributor Author

@anthonydillon Pretty straight-forward to swap out: canonical/jaas.ai#429

@nottrobin
Copy link
Contributor

Now that this is actually used in Jaas.ai, and is on the fast-track to other sites (I believe), I think we can consider it a practice. 👍 from me.

@anthonydillon
Copy link
Contributor

👍

@anthonydillon anthonydillon removed their assignment Oct 8, 2019
@nottrobin nottrobin merged commit 6562661 into canonical-web-and-design:master Oct 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants