Skip to content

Rename FixKind to FixAvailability#7658

Merged
konstin merged 2 commits intomainfrom
rename-fixkind
Oct 2, 2023
Merged

Rename FixKind to FixAvailability#7658
konstin merged 2 commits intomainfrom
rename-fixkind

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

charliermarsh commented Sep 25, 2023

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

@zanieb zanieb Sep 26, 2023

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

@konstin konstin Sep 26, 2023

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

@zanieb zanieb Sep 26, 2023

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

@charliermarsh charliermarsh Sep 27, 2023

Choose a reason for hiding this comment

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

Works for me!

zanieb
zanieb reviewed Sep 26, 2023
View reviewed changes
@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

@charliermarsh charliermarsh Oct 2, 2023

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

@charliermarsh charliermarsh charliermarsh approved these changes

@zanieb zanieb zanieb 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