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

Default ruleset discussion #300

Open
DanPurdy opened this issue Oct 13, 2015 · 4 comments
Open

Default ruleset discussion #300

DanPurdy opened this issue Oct 13, 2015 · 4 comments

Comments

@DanPurdy
Copy link
Member

After releasing 1.3.0 today @bgriffith and I were talking about the fact that our default config now has pretty much every single rule set to warn by default except no-extends I think we should discuss and decide on which rules our default config should have enabled to;

a) help someone get started with sass-lint without the extreme end of the rules affecting them
b) decide which rules are more stylistic and opinionated rather than obvious code smell..

For example I don't think force-pseudo-nesting and force-attribute-nesting should necessarily be enabled by default.

Discuss... 😄

@benthemonkey
Copy link
Member

I think that everything should be disabled by default. With all these tools (scss-lint, jshint, jscs, eslint), I always explicitly set every option I want enabled because:

  • I don't want to deal with updates changing some defaults
  • a team member may mistake a lack of setting for an undecided convention
  • the configuration is now in two places instead of one

I think an alternative would be to take eslint's lead and have an option that sets up a set of "recommended" lints.

@Snugug
Copy link
Member

Snugug commented Oct 14, 2015

Composable config like what eslint has will solve this problem, which I was slating for 2.0

On Oct 13, 2015, at 8:45 PM, Ben Rothman [email protected] wrote:

I think that everything should be disabled by default. With all these tools (scss-lint, jshint, jscs, eslint), I always explicitly set every option I want enabled because:

I don't want to deal with updates changing some defaults
a team member may mistake a lack of setting for an undecided convention
the configuration is now in two places instead of one
I think an alternative would be to take eslint's lead and have an option that sets up a set of "recommended" lints.


Reply to this email directly or view it on GitHub.

@DanPurdy
Copy link
Member Author

I think 2.0 is still a fair way off though, would it not make sense to have a stop gap for the moment? Some of the rules enabled by default currently, especially with merge default rules enabled are pretty heavy going for someone starting out. Maybe it's a case of improving the default ruleset in our documentation too?

@benthemonkey
Copy link
Member

So since you are clearly looking for a set of rules that we currently enable by default but should consider turning off, here is my take on what someone just starting to lint their Sass/SCSS might find particularly abrasive (in order from most abrasive to least):

property-sort-order - This rule will throw tons of warnings for most files. Should definitely be off by default
no-qualifying-elements - I think this is a very opinionated rule. I personally find many situations when giving a qualifying element helps express the intended use of a class.
*-format - all of the format rules are a bit extreme for the default rules
force-pseudo-nesting - I don't feel very strongly about this one, but I suspect many people don't make a habit of nesting their pseudo selectors

On an unrelated note, I just discovered scss-lint's feature where you can generate a configuration file based on the set of rules that don't throw warnings on your current codebase. Then you can slowly turn on more and more linters. Pretty cool!

@Snugug Snugug added the v2 label Feb 7, 2016
@Snugug Snugug added this to the 2.0.0 milestone Feb 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants