Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Migrate all rules from using Counter in name #60

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

Merged
merged 12 commits into from
Apr 3, 2023
Merged

Conversation

khiga8
Copy link
Collaborator

@khiga8 khiga8 commented Apr 3, 2023

Fixes: #56
Similar to #57 but for the remaining rules.

For each existing rule, this PR:

  • Deprecates the counter from the name
  • Renames the file accordingly to remove counter
  • Make the counter system configurable and not the default by wrapping the is_counter_correct? call in a conditional. It also removes the autocorrect method which previously would insert correct counter comments.

As a result of the renames, the diff is big, but the logic in the rule itself has not changed much, aside from removing the autocorrect feature and wrapping the counter call in each rule in a conditional.

I initially considered doing one PR for each rule, but decided to take of it in one go because making a PR for each rule is tedious for me, and the reviewer.

Successful test bump in dotcom

@khiga8 khiga8 marked this pull request as ready for review April 3, 2023 13:43
@khiga8 khiga8 requested a review from a team as a code owner April 3, 2023 13:43
@khiga8 khiga8 requested a review from accessibility-bot April 3, 2023 13:43
@accessibility-bot
Copy link

👋 Hello and thanks for pinging us! An accessibility first responder will review this soon.

  • 💻 On PRs for our review: please provide a review environment with steps to validate, screenshots (with alt text), or videos demonstrating functionality we should be checking. This will help speed up our review and feedback cycle.
  • ⚠️ If this is urgent, please visit us in #accessibility on Slack and tag the first responder(s) listed in the channel topic.

Copy link
Member

@owenniblock owenniblock left a comment

Choose a reason for hiding this comment

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

Looks good! ❤️

@khiga8 khiga8 merged commit e9eb4d8 into main Apr 3, 2023
@khiga8 khiga8 deleted the kh-migrate-more-rules branch April 3, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate away from Counter system
3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.