Skip to content

Conversation

@bolonio
Copy link

@bolonio bolonio commented Jul 19, 2022

Context

The motivation of erblint-github is to open-source our accessibility rules so non-GitHub people can benefit from them, have a space to provide comprehensive rule documentation, and also allow rules to be shared between Rails projects.

This PR migrates the accessibility rule a11y_link_has_href from dotcom to erblint-github

Related issue

@bolonio bolonio force-pushed the accessibility/issues/1293/migrate-a11y-erblint-rules branch from 174db85 to be03f35 Compare July 19, 2022 12:06
Copy link
Collaborator

@khiga8 khiga8 left a comment

Choose a reason for hiding this comment

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

Looks great 🚀

@khiga8
Copy link
Collaborator

khiga8 commented Jul 20, 2022

I forgot to mention this in the issue but if you're up for it, it would be good to convert this (and other non-counter rules) to a Counter as you migrate them.

The only difference implementation wise is:

  • rule_disabled? becomes counter_correct?
  • we add this def autocorrect method
  • update the rule name to end with Counter.

A Counter will always flag new violations that are added to the file (since erblint:disable completely disables the lint rule for the file). You can read more about the difference here! If you think it's out of scope we can save this for a follow-up issue as well!

@bolonio bolonio changed the title Migrate remaining accessibility rules from dotcom to erblint-github Migrate accessibility rule a11y_link_has_href from dotcom to erblint-github Jul 21, 2022
@bolonio bolonio force-pushed the accessibility/issues/1293/migrate-a11y-erblint-rules branch from 6175f26 to 8f967fa Compare July 21, 2022 09:47
@bolonio bolonio marked this pull request as ready for review July 21, 2022 09:48
@bolonio bolonio requested a review from a team as a code owner July 21, 2022 09:48
@bolonio bolonio requested review from inkblotty and owenniblock July 21, 2022 09:48
@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
Collaborator

@khiga8 khiga8 left a comment

Choose a reason for hiding this comment

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

Thanks for converting this to a counter 🎉

@bolonio bolonio merged commit 4565b54 into main Jul 22, 2022
@bolonio bolonio deleted the accessibility/issues/1293/migrate-a11y-erblint-rules branch July 22, 2022 12:14
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.

4 participants