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

Breaking: remove meta.docs.category in core rules (fixes #13398) #14594

Merged
merged 5 commits into from Aug 24, 2021

Conversation

aladdin-add
Copy link
Member

@aladdin-add aladdin-add commented May 17, 2021

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)
[ x] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

fixes #13398

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

eslint/website needs to be updated, too.

@eslint-github-bot eslint-github-bot bot added the triage An ESLint team member will look at this issue soon label May 17, 2021
@aladdin-add aladdin-add added accepted There is consensus among the team that this change meets the criteria for inclusion breaking This change is backwards-incompatible rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels May 17, 2021
@aladdin-add aladdin-add changed the title Breaking: remove category in core rules (fixes #13398) Breaking: remove meta.docs.category in core rules (fixes #13398) May 17, 2021
nzakas
nzakas approved these changes Jun 17, 2021
@mdjermanovic
Copy link
Member

mdjermanovic commented Jun 24, 2021

We should figure out the release plan for this, taking into account the following facts:

  • Our prerelease process does not modify _data/rules.yml.
  • New format of rules.yml is incompatible with the current format, meaning that the new index.liquid (eslint/website#848) must have the new rules.yml, while the current index.liquid must have the current rules.yml.

If we don't need to see the rules page on the prelease website (eslint.org/docs/8.0.0), we can:

  1. Merge this PR for the first prerelease.
  2. Merge eslint/website#848 right after the v.8.0.0 release (not prerelease).

@mdjermanovic
Copy link
Member

mdjermanovic commented Jun 24, 2021

If we want to see the rules page on the prerelease site, we could update the prerelease process to create/overwrite _data/rules<MAJOR>.yml (which would be _data/rules8.yml).

First prerelease:

  1. Merge this PR.
  2. Publish the prerelease version. The process will create _data/rules8.yml in the new format.
  3. On the website, create docs/8.0.0/rules/index.liquid file, which should be the same as in eslint/website#848 except that all references to rules must be replaced with rules8.

Release:

  1. Publish v8.0.0. The process will overwrite _data/rules.yml with the new format.
  2. Merge eslint/website#848 right after the release.

@nzakas
Copy link
Member

nzakas commented Jun 24, 2021

I think it's fine to wait until the final release before updating the website.

conf/rule-type-list.json Outdated Show resolved Hide resolved
Makefile.js Outdated Show resolved Hide resolved
Copy link
Member

@mdjermanovic mdjermanovic left a comment

LGTM, thanks!

nzakas
nzakas approved these changes Jul 1, 2021
Copy link
Contributor

@snitin315 snitin315 left a comment

LGTM

@nzakas
Copy link
Member

nzakas commented Aug 5, 2021

It looks like we have a bunch of merge conflicts here. @aladdin-add can you take a look?

chore: rm meta.cat from core rules

chore: do not require meta.cat in internal rules

chore: rename cat => ruletype

Chore: rename conf/category-list.json => rule-type-list.json

chore: update tools/update-rule-types

chore: review suggestions

Update conf/rule-type-list.json

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

Update Makefile.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
tools/internal-rules/no-invalid-meta.js Outdated Show resolved Hide resolved
@nzakas
Copy link
Member

nzakas commented Aug 12, 2021

@aladdin-add are you still working on this?

@aladdin-add
Copy link
Member Author

aladdin-add commented Aug 13, 2021

sure. seems there was something wrong when fixing the conflicts. will look later.

@aladdin-add aladdin-add requested review from nzakas and bmish Aug 13, 2021
bmish
bmish approved these changes Aug 13, 2021
nzakas
nzakas approved these changes Aug 14, 2021
Copy link
Member

@nzakas nzakas left a comment

LGTM

@mdjermanovic
Copy link
Member

mdjermanovic commented Aug 23, 2021

Any concerns about merging this? Prereleases do not update rules.yml, so this won't affect our website.

@nzakas
Copy link
Member

nzakas commented Aug 24, 2021

No concerns here.

@mdjermanovic
Copy link
Member

mdjermanovic commented Aug 24, 2021

Okay, I'm merging this so that we can publish a version where rules don't have meta.docs.category, which might be an important notice for some integrations or tools. Also to avoid future merge conflicts.

@mdjermanovic mdjermanovic merged commit 305e14a into eslint:master Aug 24, 2021
13 checks passed
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Feb 21, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove meta.docs.category from core rules
5 participants