Skip to content

Conversation

JoshuaKGoldberg
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg commented Mar 3, 2025

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: internal-only change

What changes did you make? (Give an overview)

Split out of #19354: removes all formatting-related lint rules from core. This way the main branch will not be broken by adding in formatting-related configuration settings in a different PR from the formatting changes themselves. See #19354.

Also updates tools/update-rule-type-headers.js to print tabs, since printing spaces will cause a conflict once the Prettier settings from #19354 & #19355 are applied.

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

The intended merge order of PRs to avoid main branch CI failures is:

  1. 👉 chore: remove formatting-related lint rules internally #19473 (this one): to remove any CI check that would fail on the upcoming new formatting
  2. chore: formatted files with Prettier via trunk fmt #19355: applies new formatting rules, and should be squash-merged to make a single commit for .git-blame-ignore-revs
  3. chore: enabled Prettier in Trunk #19354: adds Prettier checks in CI and as commit hooks via Trunk

@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Mar 3, 2025
Copy link

netlify bot commented Mar 3, 2025

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit aa89439
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/67cf1cb824544300080c76cc

@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review March 7, 2025 19:50
@JoshuaKGoldberg JoshuaKGoldberg requested a review from a team as a code owner March 7, 2025 19:50
@eslint-github-bot eslint-github-bot bot added the chore This change is not user-facing label Mar 10, 2025
nzakas
nzakas previously approved these changes Mar 10, 2025
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. Just need to wait for the patch release window to pass.

@nzakas nzakas moved this from Needs Triage to Merge Candidates in Triage Mar 10, 2025
@JoshuaKGoldberg JoshuaKGoldberg requested a review from nzakas March 10, 2025 17:48
@JoshuaKGoldberg
Copy link
Contributor Author

Sorry, found one last point I'd missed earlier: tools/update-rule-type-headers.js was still printing spaces in the .d.ts. Commit hooks added in the later PRs auto-switch them to tabs anyway but now that the PRs are split out & passing main, it was failing the build in #19355. Plus it's a little messy IMO to print spaces when the repo is formatted with tabs.

Anyway, I believe the three PRs are split up so that merging them sequentially won't cause failures in main. All three OPs are updated to indicate the order.

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. Thanks.

@nzakas nzakas merged commit 7a699a6 into eslint:main Mar 11, 2025
31 checks passed
@github-project-automation github-project-automation bot moved this from Merge Candidates to Complete in Triage Mar 11, 2025
@JoshuaKGoldberg JoshuaKGoldberg deleted the remove-formatting-linting branch March 11, 2025 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore This change is not user-facing
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

2 participants