Allow GitHub issue TODOs to be automatically removed #1423
Conversation
lib/sass_spec/test_case_metadata.rb
Outdated
| @@ -19,6 +19,12 @@ def self.merge_options(existing_opts, new_opts) | |||
| existing_opts[key] << v | |||
| end | |||
| existing_opts[key].uniq! | |||
| elsif key =~ /remove_((?:warning_)?todo)/ | |||
| key = $1.to_sym | |||
There was a problem hiding this comment.
This is very much a personal preference thing, but I'm not a huge fan of the elsif doing logic + the $1 fetching that logic in the next line.
Could the regex be moved to a function that returns either the key or nil? That function could be used for both checking if it's a TODO + fetching the key without having to expose the $1 to the user.
There was a problem hiding this comment.
WDYT about removed_key = key[/remove_((?:warning_)?todo)/, 1]? That way it's clear what information we're extracting, without having to create a separate function for each condition.
There was a problem hiding this comment.
Yeah, that'll work!
I just don't like $ notation because I have to backtrack through the code to understand what it's referring to.
No description provided.