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

feat!: deprecate no-new-symbol, recommend no-new-native-nonconstructor #17710

Merged
merged 3 commits into from Dec 20, 2023

Conversation

fasttime
Copy link
Member

@fasttime fasttime commented Nov 4, 2023

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[X] Other, please explain:

Fixes #17446

What changes did you make? (Give an overview)

Deprecated and unrecommended rule no-new-symbol, added no-new-native-nonconstructor to recommended rules.

Is there anything you'd like reviewers to focus on?

See the comments. The respective issue #17446 is planned for the beta phase, but I'm not sure if the idea is to merge this in v9 or earlier.

@eslint-github-bot eslint-github-bot bot added the feature This change adds a new feature to ESLint label Nov 4, 2023
Copy link

netlify bot commented Nov 4, 2023

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 1ee4c61
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/658367c8d7cf6f0008f9f10b
😎 Deploy Preview https://deploy-preview-17710--docs-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@fasttime fasttime added rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion deprecation This change deprecates an existing feature breaking This change is backwards-incompatible labels Nov 4, 2023
docs/src/rules/no-new-symbol.md Outdated Show resolved Hide resolved
lib/rules/no-new-symbol.js Outdated Show resolved Hide resolved
@fasttime fasttime marked this pull request as ready for review November 4, 2023 09:04
@fasttime fasttime requested a review from a team as a code owner November 4, 2023 09:04
@mdjermanovic
Copy link
Member

The respective issue #17446 is planned for the beta phase, but I'm not sure if the idea is to merge this in v9 or earlier.

Yes, this is planned for v9 as we can't add new rules to eslint:recommended in a minor version. I'll mark this as breaking and convert it to draft just so that we don't accidentally merge it before we start with merging v9 PRs.

@mdjermanovic mdjermanovic changed the title feat: deprecate no-new-symbol rule feat!: deprecate no-new-symbol rule Nov 4, 2023
@mdjermanovic mdjermanovic marked this pull request as draft November 4, 2023 14:02
@mdjermanovic mdjermanovic changed the title feat!: deprecate no-new-symbol rule feat!: deprecate no-new-symbol, recommend no-new-native-nonconstructor Nov 5, 2023
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Will merge for v9.0.0-alpha.

Copy link

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

@mdjermanovic
Copy link
Member

There are merge conflicts in eslint-recommended.js

@nzakas nzakas marked this pull request as ready for review December 20, 2023 22:13
@nzakas nzakas merged commit 60dea3e into main Dec 20, 2023
17 checks passed
@nzakas nzakas deleted the issue-17446 branch December 20, 2023 22:29
bmish added a commit to bmish/eslint that referenced this pull request Dec 27, 2023
* main: (25 commits)
  test: ensure that CLI tests run with FlatESLint (eslint#17884)
  fix!: Behavior of CLI when no arguments are passed (eslint#17644)
  docs: Update README
  Revert "feat!: Remove CodePath#currentSegments" (eslint#17890)
  feat!: Update shouldUseFlatConfig and CLI so flat config is default (eslint#17748)
  feat!: Remove CodePath#currentSegments (eslint#17756)
  chore: update dependency markdownlint-cli to ^0.38.0 (eslint#17865)
  feat!: deprecate no-new-symbol, recommend no-new-native-nonconstructor (eslint#17710)
  feat!: check for parsing errors in suggestion fixes (eslint#16639)
  feat!: assert suggestion messages are unique in rule testers (eslint#17532)
  feat!: `no-invalid-regexp` make allowConstructorFlags case-sensitive (eslint#17533)
  fix!: no-sequences rule schema correction (eslint#17878)
  feat!: Update `eslint:recommended` configuration (eslint#17716)
  feat!: drop support for string configurations in flat config array (eslint#17717)
  feat!: Remove `SourceCode#getComments()` (eslint#17715)
  feat!: Remove deprecated context methods (eslint#17698)
  feat!: Swap FlatESLint-ESLint, FlatRuleTester-RuleTester in API (eslint#17823)
  feat!: remove formatters except html, json(-with-metadata), and stylish (eslint#17531)
  feat!: Require Node.js `^18.18.0 || ^20.9.0 || >=21.1.0` (eslint#17725)
  fix: allow circular references in config (eslint#17752)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion breaking This change is backwards-incompatible deprecation This change deprecates an existing feature feature This change adds a new feature to ESLint rule Relates to ESLint's core rules
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

Change Request: deprecate no-new-symbol, recommend no-new-native-nonconstructor
4 participants