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

Deprecate stylelint-disable-reason #2227

Closed
jeddy3 opened this issue Dec 31, 2016 · 9 comments
Closed

Deprecate stylelint-disable-reason #2227

jeddy3 opened this issue Dec 31, 2016 · 9 comments
Labels
status: needs discussion triage needs further discussion

Comments

@jeddy3
Copy link
Member

jeddy3 commented Dec 31, 2016

Splitting #2079... next up:

  • stylelint-disable-reason

I think this rule should be a plugin because of the disconnect between it's purpose and what the implementation achieves e.g. am I right in thinking that the following will produce no warnings?

{
  "stylelint-disable-reason": "always-before"
}
/* - */
/* stylelint-disable */ 
a {}
/* stylelint-enabled */

No reason is given here, but the rule does not warn.

I think "reason" is too ambiguous to have a clear finished state.

@jeddy3 jeddy3 added the status: needs discussion triage needs further discussion label Dec 31, 2016
@davidtheclark
Copy link
Contributor

I agree 👍

@alexander-akait
Copy link
Member

@jeddy3 i can to maintain this rule, what do you think about best name for this rule?

@jeddy3
Copy link
Member Author

jeddy3 commented Jan 8, 2017

Tracking in #2123

i can to maintain this rule,

Awesome.

what do you think about best name for this rule?

I honestly don't know. You could keep it the same and document the limitations within the plugin's README?

@jeddy3 jeddy3 closed this as completed Jan 8, 2017
@jeddy3 jeddy3 mentioned this issue Jan 10, 2017
13 tasks
@kmiyashiro
Copy link

@evilebottnawi have you created a plugin for this already?

@jeddy3
Copy link
Member Author

jeddy3 commented Feb 24, 2017

@kmiyashiro If you'd like the create this plugin, and if @evilebottnawi hasn't started on it yet, then that would be awesome as the broader the community support for plugins the better for everyone :)

We've a guide to writing plugins.

For an example of converting a core rule to a plugin, you can take a look as stylelint-no-browser-hacks — which is a convert of core no-browser-hacks rule.

@kmiyashiro
Copy link

@jeddy3 Out of curiosity, why are you deprecating/removing the plugin from core instead of improving the rule or documentation? It's true that you can pass the rule without satisfying the spirit of it, but that seems to be an entirely intentional act of the developer.

@jeddy3
Copy link
Member Author

jeddy3 commented Feb 24, 2017

We recently completed an audit of the rules within stylelint. We also have our criteria for inclusion. These were developed over the last two years to create a consistent quality of rule and keep development of the core sustainable (you can search for historic discussion around this). This rule does not have a clear and unambiguous finished state, and is the only rule, I believe, that can be worked around without satisfying the spirit of it.

We have an extensible system of plugins for exactly this type of rule. The plugin author is free to experiment with trying to improve the usefulness of such a rule, unconstrained by our slower breaking-release cycle, and our desire to deliver a consistent set of rules.

I hope that explains it a little. Good luck if you do take on this plugin, both in the conversion and in improving its usefulness.

@sprynm
Copy link

sprynm commented May 15, 2018

@evilebottnawi or @kmiyashiro has a plugin been created for this yet? Seems the code is already around but I haven't got the time to maintain it myself.

@kmiyashiro
Copy link

nope, we ended up throwing it out as it seemed like too much work for a rule with dubious value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs discussion triage needs further discussion
Development

No branches or pull requests

5 participants