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.

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

feat(non-space-content): switch all non-empty checks to new generic check #2215

Merged
merged 2 commits into from
May 7, 2020

Conversation

straker
Copy link
Contributor

@straker straker commented May 6, 2020

non-empty-alt, non-empty-title, and non-empty-value were all the same function so combined them into a single generic check. Generic checks are new checks that don't have metadata files associated with them so are just evaluate functions called from other checks (can't be used directly in a rule). They typically will need options passed into them to work.

I talked to wilco about non-empty-title using titleText function and he said it didn't need to do that so was fine to just look at the title attribute directly.

Lastly, all the tests for the old checks are the exact same, especially since now they use the same evaluate underneath. I think we should remove them and just have a single test for the generic one and let the integration tests for the rules that use these checks catch things, but we should discuss that here.

Closes issue: #2192

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Code is reviewed for security

@straker straker requested a review from a team as a code owner May 6, 2020 21:44
@straker straker merged commit 7ce7b00 into develop May 7, 2020
@straker straker deleted the generic-check-non-empty branch May 7, 2020 20:32
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.

2 participants