-
Notifications
You must be signed in to change notification settings - Fork 96
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
Use reek directly #258
Use reek directly #258
Conversation
Reek::Examiner wil be run directly so we can interact with the raw output in stead of relying on the printed output which is cumbersome to process correctly.
@bf4 can you have a look at it before it's merged? |
@@ -45,7 +45,7 @@ Gem::Specification.new do |s| | |||
s.add_runtime_dependency "flay", [">= 2.0.1", "~> 2.1"] | |||
s.add_runtime_dependency "churn", ["~> 0.0.35"] | |||
s.add_runtime_dependency "flog", [">= 4.1.1", "~> 4.1"] | |||
s.add_runtime_dependency "reek", [">= 1.3.4", "~> 1.3"] | |||
s.add_runtime_dependency "reek", [">= 1.3.4", "< 2.2"] |
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.
why 2.2?
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.
Updated to 3.0, which makes more sense.
will do |
sorry, will do this weekend |
@bf4, ok to merge? |
looking at it now... |
Reek compatibility patches
Thanks! |
I'm here if you want to chat I just invited you to https://gitter.im/metricfu/metric_fu?utm_source=share-link&utm_medium=link&utm_campaign=share-link |
Ok, that's what I thought. Should be good to merge, then. |
smell_type: "Duplication", | ||
lines: [6, 9]) | ||
] | ||
@lines = double(smells: @smells) |
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.
any reason we need to make the smells double and fill with with smell doubles? I'd rather use verified doubles if we're going to do that... not a blocker, but a followup PR
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.
That would be better yeah, I'll open a new PR for it.
Merged, i think this can become 4.12? |
Yup. You good with releasing it? |
Oh, ref: using flay #231 |
Yeah I'm good with releasing it. |
This contains the commits from and thus closes #255.