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

Rename FixKind to FixAvailability #7658

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Rename FixKind to FixAvailability #7658

merged 2 commits into from
Oct 2, 2023

Conversation

konstin
Copy link
Member

@konstin konstin commented Sep 25, 2023 •
edited
Loading

Summary FixKind feels to generic, i suggest renaming it to something like FixAvailibility.

Commands used:

rg FixKind --files-with-matches | xargs sed -i 's/FixKind/FixAvailability/g'
rg fix_kind --files-with-matches | xargs sed -i 's/fix_kind/fix_availability/g'
rg FIX_KIND --files-with-matches | xargs sed -i 's/FIX_KIND/FIX_AVAILABILITY/g'
cargo fmt

rg -i "fix.kind" doesn't show any matches anymore.

@konstin konstin mentioned this pull request Sep 25, 2023
Merged
@konstin
Copy link
Member Author

konstin commented Sep 25, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@charliermarsh
Copy link
Member

I think this is more akin to "availability" -- it's an indicator of how often the rule is fixable, which is separate from the confidence of the fix itself which is the Applicability.

@konstin konstin changed the title Rename FixKind to FixConfidence Rename FixKind to FixAvailability Sep 26, 2023
@konstin
Copy link
Member Author

konstin commented Sep 26, 2023

sorry i was blind

@github-actions
Copy link
Contributor

github-actions bot commented Sep 26, 2023 •
edited
Loading

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

zanieb
zanieb reviewed Sep 26, 2023
View reviewed changes
crates/ruff_dev/src/generate_docs.rs Outdated
let fix_kind = rule.fixable();
if matches!(fix_kind, FixKind::Always | FixKind::Sometimes) {
output.push_str(&fix_kind.to_string());
let fix_confidence = rule.fixable();
Copy link
Member

Choose a reason for hiding this comment

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

Should probably update this variable name throughout

Copy link
Member Author

Choose a reason for hiding this comment

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

is FixAvailability the name we want to go with? If so i'll change everything

Copy link
Member

Choose a reason for hiding this comment

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

I like it.

Copy link
Member

Choose a reason for hiding this comment

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

Works for me!

@konstin konstin force-pushed the rename-autofix-to-fix branch from e402183 to 1addc0c Compare September 28, 2023 10:43
Base automatically changed from rename-autofix-to-fix to main September 28, 2023 10:53
@konstin konstin force-pushed the rename-fixkind branch 2 times, most recently from 7c087d3 to cded4c2 Compare October 2, 2023 12:51
@konstin
Copy link
Member Author

konstin commented Oct 2, 2023

Updated the PR description

charliermarsh
charliermarsh reviewed Oct 2, 2023
View reviewed changes
crates/ruff_linter/src/test.rs Outdated
(false, FixKind::Always) => {
panic!("Rule {rule:?} is marked to always-fixable but the diagnostic has no fix. Either ensure you always emit a fix or change `Violation::FIX_KINDd` to either `FixKind::Sometimes` or `FixKind::None")
(false, FixAvailability::Always) => {
panic!("Rule {rule:?} is marked to always-fixable but the diagnostic has no fix. Either ensure you always emit a fix or change `Violation::FIX_AVAILIBITYd` to either `FixAvailability::Sometimes` or `FixAvailability::None")
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a typo in Violation::FIX_AVAILIBITYd (which was already present before, but worth fixing).

@konstin konstin requested a review from zanieb October 2, 2023 14:28
@konstin konstin enabled auto-merge (squash) October 2, 2023 14:38
@konstin konstin merged commit 0961f00 into main Oct 2, 2023
@konstin konstin deleted the rename-fixkind branch October 2, 2023 14:38
@konstin
Copy link
Member Author

konstin commented Oct 2, 2023

Please ping me if there are any PRs that need merge conflicts due to this fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zanieb zanieb zanieb approved these changes

@charliermarsh charliermarsh charliermarsh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@konstin @charliermarsh @zanieb