WIP Feature/reek extended support #283
Conversation
| @@ -8,21 +10,18 @@ def emit | |||
| files = files_to_analyze | |||
| if files.empty? | |||
| mf_log "Skipping Reek, no files found to analyze" | |||
| @output = run!([], config_files) | |||
| run!([], config_files) | |||
There was a problem hiding this comment.
@output was used in analyze where the result from reek is passed to metric_fu structure.
This output content differ between reek versions and therefore requires different parsing to same metric_fu structure.
I moved this parsing and into different ReekExaminer classes, the ReekExaminer.get method will create proper parser depending on Reek gem version.
There was a problem hiding this comment.
I tried creating specs by making yaml fixtures storing the content of @output from the different major Reek versions (1,2,3,4).
But I fail deserialising them because I need the proper Reek classes for that version. I think I might need to serialise it to basic data types..but then I should implement the parser to work on this (meaning that we need to convert it to basic data on normal run as well)
So any suggestions on how I can verify the parsing of the Reek data structure?
ps: will push my work in progress specs as pending
|
When I was last working on MetricFu it was in preparation for a breaking change to solidify the metrics interface so that each metric could be implemented for whatever version of whatever metric gem you want. This is sort of what CodeClimate did with its engines spec, but I never got that far. I bring this up, since you are encountering those same maintenance stresses that got me thinking about how to make maintenance easier. On the plus side, this got me involved in discussions with other projects and thinking more about usability.. but.. might be time for a breaking change |
|
I think as long as we respect semver, breaking changes are ok :) |
|
Hi, I'm one of the Reek maintainers, and I strongly recommend only supporting Reek 4.x, since that is the first version that follows semver and has a defined API. |
|
Also, if you have any questions, don't hesitate to ask. I would really like to get metric_fu to support an up-to-date version of Reek, since currently it is essentially broken for me. I'd submit a pull request, but it seems there are already two of them outstanding, so I thought I'd first help sort out the versioning issues. |
…ng to Reek version. Pending specs for verifying that analyze is parsing Reek data structure to correct metric_fu common structure.
bergholdt
force-pushed
the
feature/reek-extended-support
branch
from
e988636
to
ac54433
Compare
|
@bergholdt Thanks for submitting this! Would it be possible for you to solve the conflicts here? Travis CI is now passing (✅), so I'd like to see if your changes still work. Also, #306 will bump the version of |
|
@etagwerker merged changes from master - so there is no more conflict. Perhaps it would be best to simply close this and create a new that only supports the newest Reek |
|
@bergholdt I think that is a good idea. Thank you! |
Work In Progress - need tests
This PR makes it possible to use Reek from version 1.3.4 up to current 4.1.0. and fixes Issue #272
I still need to figure out how to make automated test, but I have verified it manually running against metric_fu repo.
I wellcome any help on the test 😊