metricfu / metric_fu

A fist full of code metrics
http://metricfu.github.com/metric_fu
MIT License
627 stars 96 forks source link

Remove reek config_file_pattern option #303

Closed andynu closed 4 years ago

andynu commented 6 years ago

I couldn't install ruby 2.2.x on my laptop, so I necessarily had to add a newer ruby to the Gemfile in order to work on this pull request. Rspec tests continued to pass in 2.3.x, 2.4.x, and 2.5.x. I'd be happy to revert that change in this pull request and submit it separately.

The reek config loading changed drastically. Providing a config.reek file in the current working directory (or any of its parent directories, or your home directory) will be detected by reek. The former API that allowed metric_fu to explicitly specify the config file location is gone. I propose we adopt reek's new conventional config file finding method and abandon the explicit config_file_pattern option.

for consideration on issue #262

jkeam commented 6 years ago

Failing build :(. Check out my PR for making travis happy. I have a feeling reek is to blame for appveyor complaining.

andynu commented 6 years ago

Thank you! This is my first experience with travis; some excellent learning from your PR.

Your right about appveyor complaining because of reek (same Parser21 error as on your PR).

jkeam commented 6 years ago

Appveyor is still not happy :(

andynu commented 6 years ago

Curious. Without that adjustment activesupport, json, and rubocop weren't getting installed for me in rubies 2.3.x, 2.4.x, or 2.5.x. That is the chunk of code that I was talking about making more liberal. I was considering trying that first if condition as an else on that overall block so that activesupport, json, and rubocop are installed for all future rubies. Is there another path for making activesupport, json, and rubocop available in the bundle that I'm missing?

On Wed, Feb 28, 2018 at 9:53 AM Jonathan Keam notifications@github.com wrote:

@jkeam commented on this pull request.

In Gemfile https://github.com/metricfu/metric_fu/pull/303#discussion_r171269438:

@@ -1,7 +1,7 @@

encoding: utf-8

source "https://rubygems.org"

-if RUBY_VERSION =~ /^2.2.[0-1]p?\d/ || RUBY_VERSION =~ /^2.1.\dp?\d/ +if RUBY_VERSION =~ /^2.[3-5].\dp?\d/ || RUBY_VERSION =~ /^2.2.[0-1]p?\d/ || RUBY_VERSION =~ /^2.1.\dp?\d/

Do you need this? The tests seem to pass without it. These blocks are more of exceptional cases. Ideally we just pull from the gemspec.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/metricfu/metric_fu/pull/303#pullrequestreview-100091660, or mute the thread https://github.com/notifications/unsubscribe-auth/AADmma6VDsjpgmibXkI-Qis-BNVm-LGHks5tZWhLgaJpZM4SV69B .

andynu commented 6 years ago

The error looks to me like the old version of reek (2.2.1) is referencing a Ruby constant that is only available in Ruby 2.1. The newer versions of reek are more dynamic about the reference to the ruby parser.

I think upgrading reek will solve this.

andynu commented 6 years ago

Well, it was worth a try. No surprise that jumping two major versions of one of the metric tools will require more than tweaking a gemspec constraint. I'll look into it, but as a separate pull request. This one can be put on hold until we get things running in the newer rubies. I'd rather those land independently and then this one becomes a small feature removal.

jkeam commented 6 years ago

In regards to:

Without that adjustment activesupport, json, and rubocop weren't getting installed for me in rubies 2.3.x, 2.4.x, or 2.5.x.

I was hoping things were working bc the builds passed. Are we missing a few tests?

andynu commented 6 years ago

I guess not. There must have been something off with my Gemfile.lock when I started. I've just cleared that out and tried again without that adjusted constraint and things are bundling just fine on 2.5.0. Sorry for the false alarm.

On Wed, Feb 28, 2018 at 11:18 AM Jonathan Keam notifications@github.com wrote:

In regards to:

Without that adjustment activesupport, json, and rubocop weren't getting installed for me in rubies 2.3.x, 2.4.x, or 2.5.x.

I was hoping things were working bc the builds passed. Are we missing a few tests?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/metricfu/metric_fu/pull/303#issuecomment-369292027, or mute the thread https://github.com/notifications/unsubscribe-auth/AADmmRZbp6qp-rQaCTWS4ERWu8W7IuOcks5tZXvcgaJpZM4SV69B .

bronzdoc commented 4 years ago

@andynu can you fix the merge conflicts on this PR? thank you!

andynu commented 4 years ago

@bronzdoc I'm sorry, I do not have time to revive this two year old PR.