-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
Add matcher config in configuration file #1176
Conversation
6ab43f4
to
b8b207f
Compare
af24813
to
c899bf9
Compare
79dc889
to
53b9e40
Compare
@@ -159,13 +112,14 @@ def self.env | |||
Transform::Exception.new(YAML::SyntaxError, YAML.method(:safe_load)), | |||
Transform::Hash.new( | |||
optional: [ | |||
Transform::Hash::Key.new('coverage_criteria', CoverageCriteria::TRANSFORM), | |||
Transform::Hash::Key.new('coverage_criteria', ->(value) { CoverageCriteria::TRANSFORM.call(value) }), |
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.
I "really" like this require sequence:
require 'foo'
require 'foo/bar'
But we have to access the constant from foo/bar
here, hence as we are in class scope and the require for `foo/bar' did not happen yet we have to use a lambda indirection.
The alternative would be:
require 'foo/bar'
require 'foo'
which sucks. For the moment I eat it. Would love of Ruby had a module system ;).
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.
Would love of Ruby had a module system ;)
yeahh... I feel that one!
@@ -64,7 +64,7 @@ This prints a report like: | |||
|
|||
```sh | |||
Mutant environment: | |||
Matcher: #<Mutant::Matcher::Config match_expressions: [AUOM*]> | |||
Matcher: #<Mutant::Matcher::Config subjects: [AUOM*]> |
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.
This is a good change I think--it reads more clearly.
@@ -64,7 +64,7 @@ This prints a report like: | |||
|
|||
```sh | |||
Mutant environment: | |||
Matcher: #<Mutant::Matcher::Config match_expressions: [AUOM*]> | |||
Matcher: #<Mutant::Matcher::Config subjects: [AUOM*]> |
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.
I think this is much nicer to read. Its clear the domain of this method is matching, else the class would not be named Mutant::Matcher
.
So the match_
prefix was redundant always.
Just naming the key expressions
would IMO also suck as it than would have a sibling ignore_expressions
which does not clearly indicate what the difference between both would be.
Hence replacing this with subjects
makes it very clear. And subject_expressions
is kind of redundant as we can only match expressions.
@@ -41,6 +41,7 @@ module Mutant | |||
SCOPE_OPERATOR = '::' | |||
end # Mutant | |||
|
|||
require 'mutant/transform' |
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.
The fact this has to move up the require chain to toplevel, indicates it likely belongs to a separate library.
Once I make nicer "input error location rendering" this is worthy of a separate lib.
Transform::Hash::Key.new('fail_fast', Transform::BOOLEAN), | ||
Transform::Hash::Key.new('includes', Transform::STRING_ARRAY), | ||
Transform::Hash::Key.new('integration', Transform::STRING), | ||
Transform::Hash::Key.new('jobs', Transform::INTEGER), | ||
Transform::Hash::Key.new('mutation_timeout', Transform::FLOAT), | ||
Transform::Hash::Key.new('requires', Transform::STRING_ARRAY) | ||
Transform::Hash::Key.new('requires', Transform::STRING_ARRAY), | ||
Transform::Hash::Key.new('matcher', Matcher::Config::LOADER) |
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.
This one, paired with adding the constant Mutant::Config::LOADER
is the only real change to make config file support for matchers.
53b9e40
to
18313e2
Compare
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.
Woo! I'm excited about this one 🎉 LGTM assuming CI passes.
it should, I repushed a lot so have to manage the queue. Merging once done. |
* This class needs to traverse the process tree often, hence it tries to minimize its `marshal` representation. * For that reason we do not even use `adamantium` to avoid the memory. * A custom self freeze `.new` method was added instead. * That clashes with the one `abstract_type` installs before the re-definition of `.new`. Turning `abstract_type` semantics unobservable. * Removing abstract type for this class is the local optimum for the moment.
No description provided.