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

Changes to make for v1.0.0 [$35 awarded] #1898

Closed
btmills opened this issue Feb 28, 2015 · 36 comments

Comments

Projects
None yet
7 participants
@btmills
Copy link
Member

commented Feb 28, 2015

The following rules are deprecated and should be removed in v1.0.0:

  • generator-star
  • global-strict
  • no-comma-dangle
  • no-empty-class
  • no-extra-strict
  • no-space-before-semi
  • no-wrap-func
  • space-after-function-name
  • space-before-function-parentheses
  • space-in-brackets
  • space-unary-word-ops
  • spaced-line-comment

To remove a rule:

  1. Update the deprecation notice in docs/rules/{rule-name}.md to indicate removal.
  2. Delete lib/rules/{rule-name}.js.
  3. Delete tests/lib/rules/{rule-name}.js.
  4. Remove the rule's default setting in conf/eslint.json and any environments in conf/environments.js.
  5. Move the rule's entry in docs/rules/README.md to the Removed section.

In addition, the following behaviors are deprecated and need to be changed for v1.0.0:

  • The existing default behavior for strict with no mode option should be removed. The new default when the rule is enabled without any options should be "function". This change should be reflected in the rule's documentation.
  • In lib/api.js, stop exporting cli.

The $35 bounty on this issue has been claimed at Bountysource.

@btmills

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2015

This issue combines #1692, #1861, and #1889. @nzakas those three can be closed once you're satisfied everything from those is present here.

The existing default behavior for strict with no mode option should be removed and the default should be changed to one of "never", "global", or "function".

I'm in favor of "function" as the new default since it's most similar to the existing default. Thoughts?

@nzakas nzakas referenced this issue Feb 28, 2015

Closed

Remove deprecated strict mode rules #1692

0 of 5 tasks complete
@nzakas

This comment has been minimized.

Copy link
Member

commented Feb 28, 2015

Yup, function by default and global in the node environment.

@btmills

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2015

Yup, function by default and global in the node environment.

I've updated the issue description.

@ilyavolodin

This comment has been minimized.

Copy link
Member

commented Feb 28, 2015

I would suggest not deleting documentation for rules, and instead marking them as no longer supported with a link to the new rule. Some people might stick with the older version of ESLint for a while.

@nzakas

This comment has been minimized.

Copy link
Member

commented Feb 28, 2015

That's a good idea.

@btmills

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2015

I like it. The gensite target in Makefile.js should still work with version added stamps since it's based on the documentation files. Might we want to do something similar for the version in which a rule was removed?

@lo1tuma

This comment has been minimized.

Copy link
Member

commented Mar 6, 2015

I’ve added no-comma-dangle to the list of rules which should be removed with ESLint 1.0.

@nzakas nzakas changed the title Remove deprecated rules Changes to make for v1.0.0 Mar 7, 2015

@btmills

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2015

Added generator-star as per #1987 and space-before-function-parentheses as per #2050.

@nzakas

This comment has been minimized.

Copy link
Member

commented Mar 27, 2015

👍

@btmills

This comment has been minimized.

Copy link
Member Author

commented Apr 3, 2015

Based on #2180 (comment), will the --rulesdir option be removed in v1.0?

@btmills

This comment has been minimized.

Copy link
Member Author

commented Apr 3, 2015

The Node.js API docs state:

the cli object has been deprecated in favor of CLIEngine. It will be removed at some point in the future.

Will this be removed as part of v1.0?

@nzakas

This comment has been minimized.

Copy link
Member

commented Apr 3, 2015

I think we need to keep --rulesdir for now, probably remove in 2.0.0.

We should stop exporting cli for 1.0.0

@ilyavolodin

This comment has been minimized.

Copy link
Member

commented May 13, 2015

no-empty-class should be removed in 1.0.0 as well. It's being replaced by no-empty-character-class

@btmills

This comment has been minimized.

Copy link
Member Author

commented May 13, 2015

Updated to include no-empty-character-class.

@xjamundx

This comment has been minimized.

Copy link
Contributor

commented May 14, 2015

@btmills no-empty-class should be removed, not no-empty-character-class :)

@btmills

This comment has been minimized.

Copy link
Member Author

commented May 15, 2015

@xjamundx ...usually I can read. Fixed!

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jun 3, 2015

@btmills Please add spaced-line-comment to the deprecate rule list. As per #1088

@btmills

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2015

@gyandeeps Updated to include spaced-line-comment.

@IanVS

This comment has been minimized.

Copy link
Member

commented Jun 29, 2015

Has anyone done work on this so far? It seems pretty straight-forward, if I'm not missing something. I'm happy to start working on it if the issue is ready and accepted.

btmills added a commit that referenced this issue Jun 30, 2015

Build: gensite target supports rule removal (refs #1898)
Generated documentation will now include the version in which a rule was
removed, if applicable. I included a step to convert the `versions.json`
cache in a backwards-compatible way, so the cache won't need to be
regenerated for those of you who already have it.

Conveniently, `space-unary-word-ops` was already removed, so I used it
as a dry run and brought it in sync with the removal sequence in #1898.
@nzakas

This comment has been minimized.

Copy link
Member

commented Jun 30, 2015

These are the last tasks after the other 1.0.0 ones are complete.

Agree it would be nice to document when rules were removed. We will need to keep the docs for those rules up for a while to help those who are using older versions.

ilyavolodin added a commit that referenced this issue Jul 1, 2015

Merge pull request #2886 from eslint/issue1898
Build: gensite target supports rule removal (refs #1898)
@ilyavolodin

This comment has been minimized.

Copy link
Member

commented Jul 1, 2015

One more task, just noticed in documentation for strict rule there's a paragraph that is saying that "default" option will be deprecated in 1.0.0. Here's the link: http://eslint.org/docs/rules/strict#deprecated-mode-default

@btmills

This comment has been minimized.

Copy link
Member Author

commented Jul 1, 2015

@ilyavolodin I added that link to the strict task.

These are the last tasks after the other 1.0.0 ones are complete.

@nzakas So that I can prioritize, what order are you planning for the 1.0 tasks?

@nzakas

This comment has been minimized.

Copy link
Member

commented Jul 2, 2015

This issue is last as it's the most destructive. We also need the warning system fit missing rules to be complete before we do this.

Otherwise, everything is roughly equal in priority.

@IanVS

This comment has been minimized.

Copy link
Member

commented Jul 2, 2015

I don't see any issues with a v1.0.0 milestone for a missing rule warning system. (Nor any issue at all for it, but I haven't sifted through all 100+).

It sounds like this should be the last task that is merged. Does that mean it is also the last which should be worked on? I would love to contribute to v1.0.0 in some way, and this seems like a task I could handle and start creating a PR for.

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jul 2, 2015

@IanVS I think this is what you are looking for #1549

@nzakas

This comment has been minimized.

Copy link
Member

commented Jul 2, 2015

@IanVS Yes, this would be the last merged as it's the most destructive. It does also rely on #1549 as a way to mark rules as removed.

@gyandeeps are you still working on #1549 and #2736? Either of those might be good for @IanVS to take a look at.

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jul 2, 2015

@IanVS Feel free to pick anyone.

@nzakas

This comment has been minimized.

Copy link
Member

commented Jul 3, 2015

@IanVS do you want to look at #2736?

@IanVS

This comment has been minimized.

Copy link
Member

commented Jul 4, 2015

@nzakas Sure, should have a PR ready shortly.

@IanVS

This comment has been minimized.

Copy link
Member

commented Jul 9, 2015

It seems that coverage reports in coverage/lcov-report/lib/rules/ are not cleaned out when a rule is removed and tests are rerun. Is that possible?

Edit: I just noticed that coverage/ is in .gitignore so it doesn't matter.

@IanVS

This comment has been minimized.

Copy link
Member

commented Jul 10, 2015

I have a PR ready to go as soon as this is accepted.

@nzakas

This comment has been minimized.

Copy link
Member

commented Jul 10, 2015

Ready!

ilyavolodin added a commit that referenced this issue Jul 14, 2015

Merge pull request #2964 from IanVS/fix_1898
Breaking: Remove deprecated rules (fixes #1898)

@nzakas nzakas changed the title Changes to make for v1.0.0 Changes to make for v1.0.0 [$35 awarded] Sep 17, 2015

@nzakas nzakas added the bounty label Sep 17, 2015

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.