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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 5 commits into from Aug 24, 2021
Merged

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

merged 5 commits into from Aug 24, 2021

Conversation

aladdin-add
Copy link
Member

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

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 mentioned this pull request May 17, 2021
Merged
@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
@mdjermanovic
Copy link
Member

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 •
edited

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.

mdjermanovic
mdjermanovic reviewed Jun 29, 2021
View reviewed changes
Makefile.js Outdated Show resolved Hide resolved
mdjermanovic
mdjermanovic approved these changes Jun 30, 2021
View reviewed changes
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!

snitin315
snitin315 approved these changes Jul 4, 2021
View reviewed changes
Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

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?

@aladdin-add @mdjermanovic
Breaking: remove category in core rules (fixes eslint#13398)
152b520 
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>
@aladdin-add aladdin-add requested a review from bmish August 6, 2021 06:01
nzakas
nzakas requested changes Aug 6, 2021
View reviewed changes
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

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

nzakas
nzakas approved these changes Aug 14, 2021
View reviewed changes
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

@mdjermanovic
Copy link
Member

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

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
@MichaelDeBoey MichaelDeBoey mentioned this pull request Sep 4, 2021
Closed
2 tasks
@mdjermanovic mdjermanovic mentioned this pull request Oct 4, 2021
Merged
1 task
@rafaelss95 rafaelss95 mentioned this pull request Oct 12, 2021
Closed
4 tasks
@mtomcal mtomcal mentioned this pull request Oct 14, 2021
Merged
1 task
@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.
Reviewers

@nzakas nzakas nzakas approved these changes

@bmish bmish bmish approved these changes

@mdjermanovic mdjermanovic mdjermanovic approved these changes

@snitin315 snitin315 snitin315 approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove meta.docs.category from core rules
5 participants
@aladdin-add @mdjermanovic @nzakas @bmish @snitin315