-
-
Notifications
You must be signed in to change notification settings - Fork 200
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
Refactor html-proofer #147
Conversation
options[:verbose] = opts['verbose'] unless opts['verbose'].nil? | ||
options[:directory_index_file] = opts['directory_index_file'] unless opts['directory_index_file'].nil? | ||
options[:validate_html] = opts['validate_html'] unless opts['validate_html'].nil? | ||
options[:check_external_hash] = opts['check_external_hash'] unless opts['check_external_hash'].nil? |
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.
can this be DRY'd?
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.
ugh probably. I was thinking about removing the bin entirely but I guess that's Not The Right Thing To Do ™️.
❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️ |
Really nice. With smaller chunks we could sometimes test classes instead of the whole thing. I’ve added tests for the new Utils class in 7006132. |
Nice run. |
This refactors html-proofer into much more manageable and smaller chunks. Notably, it:
HTML::Proofer::Checks::Check
to a more saneHTML::Proofer::Runner
.It also namespaces Typheous/Hydra option passing; hence this will be a major version bump when merged.
Fixes #131, fixes #114, fixes #129.
Although none of you are under any obligation whatsoever to do so, a review from @penibelst / @benbalter / @parkr might be nice, to make sure I'm not missing something.