eslint / eslint

Find and fix problems in your JavaScript code.
https://eslint.org
MIT License
24.44k stars 4.41k forks source link

Proposal for deprecating rules #5845

Closed nzakas closed 8 years ago

nzakas commented 8 years ago

Background

ESLint has a long, depressing history of removing old rules. We did this a lot pre-1.0.0, probably longer after we should have. We stopped doing that outside of major releases after 1.0.0, which means we removed a bunch of rules in 2.0.0 as well. We rationalized this as important to keep the code base clean and we always replaced removed rules with better alternatives. We figured people could just update their configs one time and that would fine.

Problems

Now that ESLint is as popular tool, it's more important than ever that we do not break existing installations. We need to remember that whenever we're asking someone to update their config file because we removed a rule that they are doing so not just in one file, but in many, possibly hundreds of files if they use ESLint in multiple projects. That also means every shared config they rely on must be updated as well, and further means that shared configs depend very heavily on the version of ESLint they are used with.

That's a lot of pain distributed across millions of installations all to save us some pain in maintenance, and I think we're at the point where that equation is horrifically unbalanced.

Proposal

What I'd like to propose going forward is:

Maybe we could do some other things as well, I'm open to suggestions. I just think the main thing is that we need to keep rules around, deprecate them, and stop all work on them.

Thoughts? @eslint/eslint-team

ilyavolodin commented 8 years ago

In general, I agree. I'm a bit worried that after some time we would be swimming in deprecated rules, though. Maybe at some point in the future, we could add some statistic collection to ESLint and once a deprecated rule falls below a certain threshold, we could actually remove it? We should also be very careful about replacing rules/creating new rules that do almost the same thing. I assume the same applies to rule options as well?

platinumazure commented 8 years ago

@nzakas Could we have a more generous deprecation schedule? For example, deprecate a rule on 3.0.0 release, then actually remove it in 4.0.0 (or 5.0.0).

The Django project has this down to a science, although they also have a lot more infrastructure around this sort of thing. But maybe we could evaluate their model and other models and make a decision?

bdougherty commented 8 years ago

What about something like https://github.com/jamestalmage/ava-codemods to help with updating?

kaicataldo commented 8 years ago

I agree with @ilyavolodin and @platinumazure's concerns about discussing some kind of long term plan for keeping the number of deprecated rules under control. If we do implement some kind of strategy for removing deprecated rules, I think that having a long runway between deprecation and final removal, clearly setting expectations, and being very transparent about the process will be key.

I like the idea of deprecated rules hanging around for a few major versions (a set number of versions that we make very clear to the community upfront) before being removed.

pmcelhaney commented 8 years ago

The more automation the better. At a minimum print a warning when deprecated rules are in use. If possible provide a script to help update to the replacement rules.

Come to think of it, it would be nice if there was a script that introduced users to newly added rules and helped them update their configs. I might play with that idea this weekend.

lo1tuma commented 8 years ago

If a user configures a deprecated rule, can we not just map this to the replacement rule and execute it instead?

ilyavolodin commented 8 years ago

@lo1tuma Rules don't always map one to one. We had cases where we split up one rule into multiple, or combined multiple into one. It's probably possible to come up with the mapping mechanism, but it wouldn't be all that easy.

nzakas commented 8 years ago

I just don't see a way where we can remove rules, regardless of the timeframe. There are far too many references in the wild to these rules. No matter when we remove them, it will catch some people by surprise. Like it or not, we are at the center of an ecosystem here, and that means we need to be a lot more careful. Look at web browsers, they can basically never remove features.

Another reason I like this approach is that it makes it clear that we will only deprecate a rule when we absolutely need to. I think we are a bit too quick to suggest replacing or renaming a rule right now. We need to get into the mindset that doing so has a significant cost to the community and should only be attempted in rare and important cases.

What is the concern with a large number of deprecated rules?

@ilyavolodin yes, same applies to rule options.

@pmcelhaney @bdougherty a script is difficult because, by default, all the formats remove comments when parsed. Also, if you're using a shareable config, you would still have to wait until the author updated it. So a script that's smart about comments could help some folks, but the majority of our users are using a shareable config, so that alone is not enough.

BYK commented 8 years ago

@nzakas if you never remove a rule, deprecating them doesn't really mean much. People keep using them and keep complaining about bugs/features whatever you do so I'm :-1: to this new approach.

How about we try to write a small tool to map deprecated rules and their configs to new ones when possible or remove them if not from the configuration files?

nzakas commented 8 years ago

@BYK I'm getting the feeling you didn't read everything that I wrote. :)

Please see above. Deprecating means we stop work on it altogether. We don't really care if people continue to use them or not because it's no additional work for us. When people complain, we ask them to upgrade.

How about we try to write a small tool to map deprecated rules and their configs to new ones when possible or remove them if not from the configuration files?

Already explained why this is difficult. See my previous comment.

kaicataldo commented 8 years ago

@nzakas I've been thinking more about this and my initial concern of amassing a large number of deprecated rules does seem like a non-issue. My concern was more for having to manage the state of a bunch of rules (having to sort through them and spend time figuring out which rules are in what state as we're working), but as you mentioned above, this is a disproportionately small amount of pain for us compared to the large amount of pain removing the rule entirely causes.

It would be great if we could figure out a way to put them in a different directory called rules/deprecated or something...out of sight out of mind.

alberto commented 8 years ago

I agree with the underlying premise that we should be really careful before deprecating and (to a lesser extent) introducing breaking changes to rules. But never breaking rules is a big leap from that.

What you are proposing is basically "don't release major versions" anymore. I don't think anyone would be surprised by breaking changes in a major version. Not doing that means we have to get everything right designwise the first time.

I think breaking changes are ok if they are not painful. We could move deprecated rules to a different package (even each rule to a package of its own) to make keeping using them easy and at the same time, to clearly signal they are deprecated and you keep using it on your own. Also providing scripts to help with config migration would help a lot of people.

a script is difficult because, by default, all the formats remove comments when parsed

Are you talking about configuration comments and disable comments? I would guess those are a minority.

Also, if you're using a shareable config, you would still have to wait until the author updated it.

I don't see why that's a problem. Your install won't stop working in the meantime.

kaicataldo commented 8 years ago

The idea of a separate package/plugin for deprecated rules is pretty cool. Users who don't want to worry about updating their .eslintrc have to incur the upfront cost of including the deprecated rules package in their project and config, but then never have to worry about rules being deprecated again.

It also would help us in the sense that it removes the deprecated rules from core, making it very clear which rules are deprecated or not. We can also still list the rules on the site under a "deprecated" section.

SpadeAceman commented 8 years ago

We never remove old rules under any circumstances.

We no longer do any work on the rule. No bug fixes, no enhancements, […]

I just think the main thing is that we need to keep rules around, deprecate them, and stop all work on them.

At some point in the future, changes made to ESLint will eventually cause some of those old rules to stop working. There would be no point in keeping them around, since they no longer function and they're not going to be fixed.

This leaves two possible courses of action:

  1. ESLint is forever locked into supporting all deprecated rules indefinitely (which seems like a bad idea).
  2. At a minimum, there would need to be some kind of clearly-documented process for old rule removal due to incompatibility.

Either way, this implies continual testing of all deprecated rules, to ensure that they still work in future releases of ESLint.

Alternatively, deprecated rules can come with a "caveat emptor" understanding – people use them at their own risk, and they might break at any time without warning. However, this undermines the backwards compatibility intended by keeping those old rules around.

In my opinion, @platinumazure's recommendation of having a generous deprecation schedule seems best. This provides a clear timeline for old rule removal (rather than sudden rule disappearance due to a breaking change), and ensures that any future ESLint design changes which break compatibility with deprecated rules can be scheduled appropriately.

platinumazure commented 8 years ago

Incidentally, we might have some test cases if we wanted to try a particular process. For example, no-with or no-continue could be deprecated/removed in favor of no-restricted-syntax. I assume neither of those rules has received an update in a while. Perhaps we could put together a couple of specific proposals using those rules as examples and then allow the community to provide feedback?

BYK commented 8 years ago

@nzakas - I read all your comments before writing mine. I understand why you would think that though so let me expand on my opinion a bit more. On removing, I don't think us mentioning that those rules are not being worked on and even not working on any bug reports would stop people from complaining. Automatically closing issues using a bot has the following problems:

  1. It is still hard to be accurate, I'm pretty sure there will be issues seeping through the cracks
  2. It doesn't stop me from getting e-mails or notifications for those issues so it is still noise.
  3. It doesn't stop complaints coming from other channels such as the mailing list, Gitter or Twitter.

Removing these rules almost completely solves/avoids these problems without any work on a smarter bot. Also it makes deprecating rules a lot more meaningful, just like Firefox does with experimental features and vendor-prefixed things. I don't wanna end up where Chrome ended up since now they have to support a lot of crap forever due to same fears. Didn't really help anyone.

For the automatic conversion of config files, I understand preserving comments are hard but it shouldn't be impossible. JSON files have no comments so that's a non issue. For JS files, we have the CST and similar things (even Espree preserves comments) but there may be other complexities there, I agree. For YAML I found this project: https://github.com/mulesoft-labs/yaml-ast-parser. not sure if it preserves comments yet but it at least demonstrates that what we want to achieve is not incredibly hard if we really wanted to.

I am still not convinced about us not dropping any rules, ever. Even large frameworks deprecate stuff and remove them in the next version and I don't think ESLint is that special in terms of ecosystem.

nzakas commented 8 years ago

Okay wow, so I think there's some pretty big misunderstandings going on here, as people are summarizing my proposal in a bunch of ways that are incorrect. Let me try to clear things up by answering individual points.

@kaicataldo @alberto

The idea of a separate package/plugin for deprecated rules is pretty cool. Users who don't want to worry about updating their .eslintrc have to incur the upfront cost of including the deprecated rules package in their project and config, but then never have to worry about rules being deprecated again.

We can't actually do this for a few reasons. First, plugins always have prefixes, so not only would people have to download the package, they would still have to update their config files, which is exactly the pain I was looking to avoid with this proposal. Second, we'd still need to keep the documentation for deprecated rules along with all the other rules, so separating that out would also cause confusion.

@alberto

What you are proposing is basically "don't release major versions" anymore. I don't think anyone would be surprised by breaking changes in a major version. Not doing that means we have to get everything right designwise the first time.

This is not what I am saying. First, major releases are not strictly tied to rules, we can have other breaking changes that would require a major release (such as dropping support for Node < 4). And again, it's not that people will be surprised about breaking changes, I don't really care how surprised people are, what I care about is the pain people feel as a result of breaking changes. Needing to update every config file they have across dozens of repositories is a significant pain. Also, we don't have to get things right the first time. We can always deprecate a rule we get wrong and replace it with something else. The difference is that we are not penalizing all of our users because we made a mistake.

Also, if you're using a shareable config, you would still have to wait until the author updated it.

I don't see why that's a problem. Your install won't stop working in the meantime.

Yes it will. If we removed a rule that the shareable config includes, you cannot upgrade until that shareable config has been updated as well.

a script is difficult because, by default, all the formats remove comments when parsed

Are you talking about configuration comments and disable comments? I would guess those are a minority.

I'm talking about comments in .eslintrc files, but you're right, configuration comments and disable comments are problematic, too. And I don't think we can make any guesses as to whether they are common or not.

@SpadeAceman

At some point in the future, changes made to ESLint will eventually cause some of those old rules to stop working.

This is completely untrue. Old rules must continue to work because people are relying on custom rules. We simply cannot ever make a change that would cause any existing rules to break.

@BYK

just like Firefox does with experimental features and vendor-prefixed things.

There is a huge difference between removing experimental features and vendor-prefixed things vs. removing ESLint rules: the browser features were intentionally introduced as short-term fixes for problems, and that was clearly communicated to the user base. In our case, we clearly communicated that the rules are safe to use, so we are breaking a contract with our users (whereas Firefox is enforcing a contract).

For the automatic conversion of config files, I understand preserving comments are hard but it shouldn't be impossible.

You are still missing the part about how most users are using shareable configs that would also have to be updated. Picture this: you are using eslint-config-google plus two of your own config files. You decide to update to ESLint 3, which removes some rules that are mentioned in all three config files (two of yours, one that is shareable). You are now stuck on upgrading waiting for eslint-config-google to be updated. That's a lousy situation to be in and you have no way to fix it yourself.

I don't think ESLint is that special in terms of ecosystem.

This is our main disagreement. :) ESLint is integrated into people's build systems and editors. When ESLint breaks, people cannot commit or release their code. That's a responsibility I take very seriously.

IanVS commented 8 years ago

In the process of deprecating a rule, would we remove/disable its tests?

nzakas commented 8 years ago

Starting a new comment for my own sanity.

So far, the only downside anyone has identified for not removing rules is that people will still file issues about the deprecated rules, and we'll have to close them.

I'd also like to point out a few additional things:

  1. In the nearly three years of ESLint's existence, we have only removed 18 rules (we have over 200 right now), so basically 6 rules per year. For 2.0, we removed five rules and the rest were removed in 1.0.
  2. Removing rules is a way to minimize maintainer pain, but doing so creates a disproportionate amount of user pain.
  3. Breaking changes that require users to update configuration files inflict the largest amount of pain possible because configuration files live in each repo and are included via shareable configs. Breaking changes that do not require users to change config files can more easily be automated.

The only counter-proposal that shows a good understanding of these concepts is @platinumazure's proposal for an extended deprecation cycle. However, any extended deprecation cycle would have to be time-bound and not release-bound. So if we, for instance, said that deprecated rules are removed a year after their deprecation, then we'd still have the problem of people filing issues against those rules for a year. (And I think anything less than a year is too short notice for most people.) So is that tangibly better than just not removing rules?

I'm happy to keep discussing, but I feel like this topics have already been discussed and been shown to be unrealistic:

  1. Automatic updating of configs - this would be a lot of work for us to get right, and even then, the shareable config problem remains. I don't think this is a viable solution.
  2. Splitting out deprecated rules into a plugin - moving rules into a plugin requires all of the same changes as removing rules from ESLint altogether. We can always rearrange the rules internal to the code base if it makes it easier for us, but completely splitting out into a separate plugin doesn't solve any part of the problem.
nzakas commented 8 years ago

@IanVS no, we need the tests to remain so we can verify if we broke anything.

IanVS commented 8 years ago

I agree we need to be very conscious of how our actions affect lots of people, and would be in favor of a one year deprecation timeline, perhaps combined with some helpful tooling. "Forever" is a very long time, which I think is why some of us are nervous about stagnation or lock-in. I agree that tooling can't automatically update configs, but it could at least identify deprecated rules that a user is relying on and suggest an upgrade path. Maybe something like below (could include links to more details, too).

eslint --check-deprecated ./

no-arrow-condition       is replaced by a combination of no-confusing-arrow and no-constant-condition and is scheduled for removal in Jan, 2018. Turn on both of these rules to get the same functionality as no-arrow-condition.
no-empty-label           is replaced by no-labels with {"allowLoop": true, "allowSwitch": true} option and is scheduled for removal in Oct, 2017.
space-after-keywords     is replaced by keyword-spacing and is scheduled for removal in Dec, 2017.
space-before-keywords    is replaced by keyword-spacing and is scheduled for removal in Dec, 2017.
space-return-throw-case  is replaced by keyword-spacing and is scheduled for removal in Dec, 2017.
platinumazure commented 8 years ago

The only counter-proposal that shows a good understanding of these concepts is @platinumazure's proposal for an extended deprecation cycle. However, any extended deprecation cycle would have to be time-bound and not release-bound. So if we, for instance, said that deprecated rules are removed a year after their deprecation, then we'd still have the problem of people filing issues against those rules for a year. (And I think anything less than a year is too short notice for most people.) So is that tangibly better than just not removing rules?

I asked myself, why time-bound rather than release-bound? Then I realized that the Django project (which you'll recall had originally inspired my proposal) does things in a release-bound way, but their releases are fairly rigorously scheduled, so in a way, they are time-bound.

Personally, I think a year is longer than needed and that 6 months would be reasonable in most cases, though we could certainly err on the side of a longer deprecation period for more complex deprecations. a year might be sufficient time, though we could also consider a longer period.

As for people filing issues against those rules, yes, that is a weakness of my proposal, and to be honest it was something I had not intended to address. What I was trying to do with my proposal is to reduce the maintenance pain of keeping rules around forever while also mitigating the user pain of upgrading rules. As was pointed out by @nzakas, we deprecate maybe 6 rules a year (out of 200+), so my six-month proposal implies having to manage 1 rule a month. Actually, now that I'm doing this math I can see an argument for a year-long deprecation period.

Incidentally, in order to help encourage people to handle deprecated rules, we could conceivably emit a warning (not an error) every time the deprecated rule is used. (Ideally, this would not count against the --max-warnings flag.) Taking more inspiration from Django, one of their deprecation tiers is that warnings will be emitted loudly without any command line flags. They also have a tier in which warnings are emitted with a command line flag only, so that people can look ahead and handle deprecations even more quickly.

I think maybe a two-tiered deprecation could work for us, too. Six months of silent deprecation (and a command line switch to show warnings for using rules in this tier), followed by six months of loud deprecation (no command line switch needed to show warnings for using rules in this tier), followed by removal. In order to comply with semver, we could agree that the six-month timing is imprecise and we just align the change in status with major releases, trying to guarantee at least six months in each tier if we can. (Obviously, this proposal could be adapted to reflect different durations at each deprecation level.)

@nzakas Any thoughts on this?

ilyavolodin commented 8 years ago

I'm not too worried about people filing issues against deprecated rules, honestly. I just don't really want to drag that baggage forever. But it looks like you feel very strongly about this, and my position is pretty flexible, so I'm not going to oppose this.

alberto commented 8 years ago

@nzakas I don't want to get too argumentative, so feel free to ignore the responses below, but since you were asking for feedback, you could keep the discussion open a little longer before discarding ideas as unrealistic :)

We can't actually do this for a few reasons. First, plugins always have prefixes, so not only would people have to download the package, they would still have to update their config files, which is exactly the pain I was looking to avoid with this proposal.

Couldn't it be feasible to make an exception for our eslint-plugin-deprecated-rules package so that they didn't need to be prefixed? Then one would only have to include that.

Second, we'd still need to keep the documentation for deprecated rules along with all the other rules, so separating that out would also cause confusion.

I don't see why they couldn't be still be generated on our website.

This is not what I am saying. First, major releases are not strictly tied to rules, we can have other breaking changes that would require a major release (such as dropping support for Node < 4). And again, it's not that people will be surprised about breaking changes, I don't really care how surprised people are, what I care about is the pain people feel as a result of breaking changes. Needing to update every config file they have across dozens of repositories is a significant pain. Also, we don't have to get things right the first time. We can always deprecate a rule we get wrong and replace it with something else. The difference is that we are not penalizing all of our users because we made a mistake.

Here I was thinking also about breaking changes in options too. The proposal is centered in deprecating rules but the underlying one is (correct me if I am wrong) "don't do anything that would cause end users to have to update their configs".

I think we should allow ourselves to fix our stuff and do some cleanup once in a while. I agree we should limit that, and I'm ok with one year or whatever we agree is reasonable.

pmcelhaney commented 8 years ago

One thing that bothers me about ESLint is when it doesn't recognize a rule it fails on every single file.

Can we just change the error to a warning and skip over the unrecognized rule?

And if the rule happens to be one that was deprecated and removed, add a bit more detail in the warning message, as Van suggests?

On Apr 18, 2016, at 2:49 PM, Ian VanSchooten notifications@github.com wrote:

I agree we need to be very conscious of how our actions affect lots of people, and would be in favor of a one or even two year deprecation timeline, perhaps combined with some helpful tooling. "Forever" is a very long time, which I think is why some of us are nervous about stagnation or lock-in. I agree that tooling can't automatically update configs, but it could at least identify deprecated rules that a user is relying on and suggest an upgrade path. Maybe something like below (could include links to more details, too).

eslint --check-deprecated ./

no-arrow-condition is replaced by a combination of no-confusing-arrow and no-constant-condition and is scheduled for removal in Jan, 2018. Turn on both of these rules to get the same functionality as no-arrow-condition. no-empty-label is replaced by no-labels with {"allowLoop": true, "allowSwitch": true} option and is scheduled for removal in Oct, 2017. space-after-keywords is replaced by keyword-spacing and is scheduled for removal in Dec, 2017. space-before-keywords is replaced by keyword-spacing and is scheduled for removal in Dec, 2017. space-return-throw-case is replaced by keyword-spacing and is scheduled for removal in Dec, 2017. — You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

BYK commented 8 years ago

@nzakas - Okay, I think I didn't understand your underlying concerns before. Now I'm almost a convert. I understand shared configs may be an issue. I think if we can find a way to automatically update configs (which is already not very easy) we can also get all repos with ESLint configs and submit automated PRs to them as other projects do. We can also encourage shared config maintainers to mention ESLint with a proper version as a peerDependency too (we should do this right now too).

I still feel like carrying that old rule baggage around with us too long is not great but I'm not opposed to your idea since I now understand your reasons better.

SpadeAceman commented 8 years ago

One thing that bothers me about ESLint is when it doesn't recognize a rule it fails on every single file.

Can we just change the error to a warning and skip over the unrecognized rule?

And if the rule happens to be one that was deprecated and removed, add a bit more detail in the warning message, as Van suggests?

Or even better – why not just turn the deprecated rules into "no-op" modules?

The proposal is centered in deprecating rules but the underlying one is (correct me if I am wrong) "don't do anything that would cause end users to have to update their configs".

If the goal is to keep user configs from breaking, does it matter if the old rules actually do anything after they're deprecated?

Perhaps all a deprecated rule needs to do is log a message about how it's been deprecated and no longer does anything, please use rule X instead. This avoids breaking legacy configs, provides a clear path forward for the user, prevents problematic bug reports/feature requests from being submitted for the deprecated rules, and keeps ESLint from having to support them indefinitely.

scriptdaemon commented 8 years ago

I like the idea of printing a non-failing warning for each deprecated rule (and their replacement) being used during linting, FWIW. Without something of that sort, it's hard for a user to know what rules have been deprecated and what rule(s) replaced them without going through the changelog (which I'm willing to bet a lot of users don't do).

nzakas commented 8 years ago

Some data from an informal poll on Twitter: https://twitter.com/geteslint/status/722135012955410432

tl;dr 12% said that removing rules is horrible and wish we wouldn't do it.

nzakas commented 8 years ago

Okay, there are a few streams going on here, so let me try to unpack them.

I think the overall point I want to make about various alternatives discussed is that most of them require us to do a nontrivial amount of work to support it, whereas my proposal takes basically zero work (outside of documenting things). So, keep that in mind. :)

@alberto

Couldn't it be feasible to make an exception for our eslint-plugin-deprecated-rules package so that they didn't need to be prefixed? Then one would only have to include that.

One-off exceptions are almost always a bad idea. The reason we have prefixes is to avoid naming collisions, which would also be a concern here. We'd need to always include the special package with ESLint anyway, since we couldn't expect people to manually install it everywhere, which kind of defeats the purpose. Overall, I don't think this buys us much.

I don't see why they couldn't be still be generated on our website.

Yes, of course, we are smart people and can figure out a way to make this work. But why do all this extra work for six rules per year (if that)? This would make a release a lot more fragile because we'd be dealing with another repo.

Here I was thinking also about breaking changes in options too. The proposal is centered in deprecating rules but the underlying one is (correct me if I am wrong) "don't do anything that would cause end users to have to update their configs".

Rule options and rules themselves should be treated as the same (pretty sure @ilyavolodin was the first person to say this, and I agree). We should definitely stop making breaking changes to rule options. I think we've generally been pretty good about this after 1.0 came out, so I don't see this as very controversial.

I think we should allow ourselves to fix our stuff and do some cleanup once in a while.

Again, we can fix stuff by adding new rules and deprecating old ones. There's nothing about my proposal that prevents us from fixing our own mistakes. The only difference is that there's an artifact of the original still left around for people who don't upgrade.

@pmcelhaney

One thing that bothers me about ESLint is when it doesn't recognize a rule it fails on every single file.

We kind of need to do this, as far as I can tell, since rules can be enabled individually in files as well as through config. But this is really tangential from the main discussion, as you're talking about what we do when we remove files, and I'm suggesting we just don't remove files at all. :)

@BYK

I think if we can find a way to automatically update configs (which is already not very easy) we can also get all repos with ESLint configs and submit automated PRs to them as other projects do.

Again, a TON of work that doesn't really solve much. So yes, we could do this for configs on github.com and probably annoy a lot of people. But what about projects on gitlab.com? Or BitBucket? What about configs that are in private repos? We just don't get anywhere near enough coverage for the amount of effort this would take.

@SpadeAceman

Or even better – why not just turn the deprecated rules into "no-op" modules?

Because then we are also breaking people's experiences. The goal here is to not break what people are already doing so they don't have to update their config files.

If the goal is to keep user configs from breaking, does it matter if the old rules actually do anything after they're deprecated?

Of course it does! If you have a rule enabled that checks for some sort of possible error, and all of a sudden that rule does nothing, that is a breaking change (and arguably a high sin for a linter).

nzakas commented 8 years ago

I feel like we have all kinds of viewpoints represented in this thread now, so here's what I'd like to propose: let's try not removing deprecated rules for the next year (essentially, @platinumazure's proposal). Here's my thinking:

  1. The problem of "too many deprecated rules" doesn't currently exist (and we don't know if it ever will).
  2. The only way to get data about this is to actually try it out.
  3. The cost of trying it for a year is negligible as opposed to the cost of trying some of the other proposals thrown around in this thread.
  4. If we end up deprecating some rules, we can figure out the right way to notify end users of that change.
  5. At the end of the year, we'll have actual data on which we can make a decision.
  6. At that point, we can either continue not deleting deprecated rules or we can delete them, based on our experiences.

Again, I strongly believe that at this point we should take breaking changes to rules extremely seriously and not do them if at all possible, and that in turn means we'd likely not have that many rules deprecated. The primary question seems to be how much pain that will cause for us vs. the pain that it would cause to developers if we removed the rules, and I think spending the next year gathering such data would be a better use of time than anything else.

Thoughts?

pedrottimark commented 8 years ago

Merging with JSCS means we will probably be too busy to deprecate rules for a year, anyway.

A more imminent challenge seems like verifying which rules are equivalent and dealing with rules that are similar but not quite the same in a way that people can understand. Converging when oh-so-close might be where there is risk to break and temptation to deprecate.

alberto commented 8 years ago

Again, we can fix stuff by adding new rules and deprecating old ones. There's nothing about my proposal that prevents us from fixing our own mistakes. The only difference is that there's an artifact of the original still left around for people who don't upgrade.

I don't care so much about deprecating rules, I was just proposing alternatives in case they were viable, so I am not pushing this any further. :)

I am more concerned about not doing any breaking change EVER:

scriptdaemon commented 8 years ago

I understand this would be considered an unnecessary breaking change and probably won't ever happen, but I really like the consistent wording for JSCS rules. If you ever change your mind and allow rules to be renamed (even as a one-time overall change), I think several rule titles could use a bit of an overhaul. There are several that aren't clear as to their purpose (e.g. max-statements, especially with the addition of max-statements-per-line). Plus, I ran into a few instances where some rule options took boolean values in one rule, but options of the same name took a string in other rules.

nzakas commented 8 years ago

@alberto all of your concerns can be better served by observing over time rather than guessing about potential problems. Right now, you fear that these things might happen, but they might never happen. This is really the point I've been trying to make to you: removing rules causes a lot of real, tangible pain that we know exists. Not removing rules might potentially cause some pain (to us), but it might not...and if it does, then we can deal with it as it comes up. And if that happens, I'll happy tell everyone that you were right and I was wrong, and at that point we'd know that it's worth the effort to invest in more tooling, processes, etc. to mitigate it.

If we want to do a breaking change to a rule and we deprecate it we have to come up with a new name, so it's not cost free for us.

This cost is tiny compared to the cost of inconveniencing everyone using ESLint. Copy the file over, create a new name, the cost is small and fixed. Again, the cost to try to my approach starts at zero and goes to small whereas the alternative approaches people suggested start at medium pain and go to large.

@scriptdaemon after a certain point, changing the names of rules creates more confusion rather than less. But again, this is completely tangential to the topic at hand.

nzakas commented 8 years ago

I also want to highlight: regardless of my proposal here, we should not even consider introducing breaking changes to rules unless not doing so creates more harm than good. I can't highlight this enough: there are literally millions of ESLint installs running around the world in all kinds of environments that might not be easy to update. I personally went through a lot of pain getting all of the ESLint organization repos up to date when we released 2.0 due to eslint-config-eslint referring to new rules when other repos weren't yet updated. This pain is very real, and I will gladly throw myself in front of a train to prevent our users from needing to go through this unless it's absolutely necessary.

alberto commented 8 years ago

Just to be clear, I am all for your proposal of not introducing breaking changes for a year. It's not about me trying to be right, again, it is just I fear we announce we won't ever introduce breaking changes, because once you do that, you "can't" really go back.

scriptdaemon commented 8 years ago

@nzakas I should have been more clear (as to the reason I thought it was relevant) that my idea for something like that is to create a new rule with the new wording and deprecate the rule with the old wording. Moot point, but just thought I'd clarify.

nzakas commented 8 years ago

Okay, to circle back so we can get a conclusion here, the current proposal is:

@eslint/eslint-team please indicate that you've read this comment and agree with this proposal moving forward by adding a :+1:.

xjamundx commented 8 years ago

👍🏼

On Fri, Apr 29, 2016 at 10:52 AM Nicholas C. Zakas notifications@github.com wrote:

Okay, to circle back so we can get a conclusion here, the current proposal is:

  • For the next year, we will not remove any rules (though we will deprecate as necessary)
  • Instead, we deprecate rules that we believe people shouldn't use anymore. When a rule is deprecated, it means:
    1. The rule is moved to the "deprecated" page (maybe eslint.org/docs/rules/deprecated/) instead of being on the main page.
    2. We label the rule as deprecated on the rule's documentation page.
    3. We no longer do any work on the rule. No bug fixes, no enhancements, no documentation updates (except for website formatting changes to coincide with other design changes). Any requests are automatically closed with a message to use the replacement rule(s) instead.
  • After a year has passed, we will evaluate whether it makes sense to remove the deprecated rules or not.

@eslint/eslint-team https://github.com/orgs/eslint/teams/eslint-team please indicate that you've read this comment and agree with this proposal moving forward by adding a [image: :+1:].

— You are receiving this because you are on a team that was mentioned. Reply to this email directly or view it on GitHub https://github.com/eslint/eslint/issues/5845#issuecomment-215829286

alberto commented 8 years ago

:+1:

ilyavolodin commented 8 years ago

:+1:

mikesherov commented 8 years ago

:+1:

Mike Sherov

On Apr 29, 2016, at 3:44 PM, Ilya Volodin notifications@github.com wrote:

— You are receiving this because you are on a team that was mentioned. Reply to this email directly or view it on GitHub

alberto commented 8 years ago

@nzakas can we consider this accepted? Any further steps with this (should this be documented?) or shall we close it?

nzakas commented 8 years ago

Oh yeah, we should document this. Probably in the user guide?

kaicataldo commented 8 years ago

Looks like we'll also want to update the site with the /docs/rules/deprecated/ page and move the list of "Removed" rules there.

nzakas commented 8 years ago

@kaicataldo the removed rules shouldn't be grouped with the deprecated rules, as those are two different things. We should ask @pedrottimark what he thinks about having a separate page vs. a new section. We don't have any deprecated rules right now, so this is all future work.

kaicataldo commented 8 years ago

@nzakas Got it. Just wanted to make sure we don't reference a page that doesn't exist in our docs :)

platinumazure commented 8 years ago

@nzakas Could we extend the meta section of new-style rules to include a deprecation date? If so, we could then enhance core to automatically warn or error for deprecated or "removed" rules. We could also add other fields for things like reason why deprecated, or replacement rule(s), etc.

Would that be worth doing? We could try to "retrofit" for existing removed rules by recreating metadata-only versions of those rules, and we could test out the proposed deprecation process once this is in place.

pedrottimark commented 8 years ago

Will put information architecture for future deprecated process into my mental slow cooker. While retrieving data for the following point, I got a taste of how it affected the rules index in the past.

At this very moment (related to #5774 to automatically generate the rules index page) I am making a prototype of a file to contain:

If I understand correctly, Makefile.js already gets added and removed versions from commit history caches them in a versions.json file on your local file system.