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

Only show warnings for empty preview selectors when enabling rules #7842

Merged
merged 2 commits into from
Oct 8, 2023

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Oct 6, 2023

Closes #7491

Users found it confusing that warnings were displayed when ignoring a preview rule (which has no effect without --preview). While we could retain the warning with different messaging, I've opted to remove it for now. With this pull request, we will only warn on --select and --extend-select but not --fixable, --unfixable, --ignore, or --extend-fixable.

@zanieb zanieb added the preview Related to preview mode features label Oct 6, 2023
if preview.mode.is_disabled() {
// Only warn for the following selectors if used to enable rules
// e.g. use with `--ignore` or `--fixable` is okay
if preview.mode.is_disabled() && kind.is_enable() {
if let RuleSelector::Rule { prefix, .. } = selector {
if prefix.rules().any(|rule| rule.is_nursery()) {
deprecated_nursery_selectors.insert(selector);
Copy link
Member Author

Choose a reason for hiding this comment

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

We could also push kind to these warning lists to facilitate different warnings by kind.

Comment on lines +66 to +67
/// Modifies the behavior of selected rules
Modify,
Copy link
Member Author

Choose a reason for hiding this comment

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

Could be made more specific in the future if needed 🤷‍♀️

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

LGTM!

@zanieb zanieb merged commit 2d6557a into main Oct 8, 2023
16 checks passed
@zanieb zanieb deleted the zanie/preview-warn branch October 8, 2023 16:14
@bersbersbers
Copy link
Contributor

While we could retain the warning with different messaging, I've opted to remove it for now.

This is the right choice IMHO. Thank you!

konstin pushed a commit that referenced this pull request Oct 11, 2023
…7842)

Closes #7491

Users found it confusing that warnings were displayed when ignoring a
preview rule (which has no effect without `--preview`). While we could
retain the warning with different messaging, I've opted to remove it for
now. With this pull request, we will only warn on `--select` and
`--extend-select` but not `--fixable`, `--unfixable`, `--ignore`, or
`--extend-fixable`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Related to preview mode features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve warnings when ignoring preview rules without preview flag
3 participants