- Notifications
You must be signed in to change notification settings - Fork5
feat: finalizes interface for consuming codebases#4
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
👋 Hello and thanks for pinging us! An accessibility first responder will review this soon.
|
| Consider upstreaming any rules you find useful as proposals to this repository. | ||
| ## Development |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
is this header supposed to be empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
there's no text before the lower-level heading starts, that part is intentional. if it reads as unintentional tho I can fix that. maybe something better than "functional troubleshooting" as the first content within it. will massage it, thx for the tip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
oh your comment just made me realize I totally missed the extra sharp sign and this is actually functional heading structure 🤦♀️ my bad!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
my philosophy is: if it made you skip a beat, then your confusion is valid. worth noting and finding solutions, even if what i have here is technically correct. clarity is my aim!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
done.
| const noDefaultAltText = require('./no-default-alt-text') | ||
| const customRules = [noDefaultAltText] | ||
| const customRules = [ | ||
| noDefaultAltText |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
note: content of function needs to match what we're removing from the Hub, seehttps://github.com/github/thehub/blob/a1a6e61ac6ee06e362a4b7d84be73faeed8cddd2/.github/linters/accessibility-lint-rules/no-default-alt-text.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Uh oh!
There was an error while loading.Please reload this page.
Relates to github/accessibility#1591
To reviewer: This waits only for usage tests.
I'm working on this alongside changes in github/accessibility and github/thehub. We'll merge here first, then do accessibility, and finally The Hub.
note to self todo: confirm that Owen's final changes in The Hub made it into our code+tests.