Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add matcher config in configuration file #1176
Changes from 1 commit
18313e2
b069361
cbe30c2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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 siblingignore_expressions
which does not clearly indicate what the difference between both would be.Hence replacing this with
subjects
makes it very clear. Andsubject_expressions
is kind of redundant as we can only match expressions.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.
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:
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:
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.
yeahh... I feel that one!
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.This file was deleted.