Skip to content

Conversation

@JoshuaKGoldberg
Copy link
Contributor

Fixes #165.

I felt unsure labeling this as "feat" vs. "fix" - the root behavior is, arguably, a bug fix for feature support of rule.meta.defaultOptions... 🤷

Code roughly copied from eslint/eslint#17656.

nzakas
nzakas previously approved these changes Jul 3, 2024
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.

Overall LGTM. Just one note about syntax.

Would like @mdjermanovic to review before merging.

nzakas
nzakas previously approved these changes Jul 3, 2024
@nzakas
Copy link
Member

nzakas commented Jul 17, 2024

@eslint/eslint-tsc This PR is still looking for another review.

Copy link
Member

@fasttime fasttime left a comment

Choose a reason for hiding this comment

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

Sorry for the late review. I just left a few suggestions. Other than that, I think this looks good.

Co-authored-by: Francesco Trotta <[email protected]>
@JoshuaKGoldberg JoshuaKGoldberg requested a review from fasttime July 26, 2024 15:41
Copy link
Member

@fasttime fasttime 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! Pending a last review from @mdjermanovic.

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.

This LGTM, but I think we should hold off merging until eslint/eslint#17656 is approved to make sure the changes are synchronized.

@mdjermanovic mdjermanovic merged commit d02f914 into eslint:main Nov 14, 2024
@github-actions github-actions bot mentioned this pull request Nov 14, 2024
@JoshuaKGoldberg JoshuaKGoldberg deleted the factor-in-default-options branch November 14, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

Fix: Validate options after applying defaults, not before

5 participants