Decouple measuring of results from reporting results using formatters.

[robincurry]

The idea here is to use similar approach that you see in libraries like simplecov, rspec or cucumber, where you can specify different formats (either pre-built or custom) from the command-line.

So, you could do something like:

metric_fu -r --format json --out '/path/to/output.json'

or

metric_fu -r --format html --format csv --out '/path/to/my.csv'

or perhaps even something like...

metric_fu -r --format redis

I think it opens up some interesting possibilities.

My intent is to keep everything backward-compatible (and I think so far, so good), so running metric_fu without specifying a format will use the built-in html formatter which uses the awesome templating / graphing stuff and produces your standard metric_fu report.

[bf4]

yes. feel free to switch out the internals of the cli option parsing however you like

[bf4]

In terms of directory structure as we move forward, here are my thoughts beyond what I've written elsewhere Also see my config PR and grapher PR

 lib/
    metric_fu.rb
    metric_fu_requires.rb
    metric_fu/
                   configuration.rb # contains MetricConfig
                     # which contains all the configuration info for each metric
                     # is in a collection of configured metrics in the Configuration
                     # has :configure method that sets the metric_name, metric_conf, if gaphed, when skipped/activated
                     # MetricFu::ChurnConfig
                   metric.rb # was generator, is interface for running, analyzing, formatting, outputting metrics
                   metrics/
                     MetricFu::Churn < MetricFu::Metric
                       churn/ 
                          config.rb # MetricFu::ChurnConfig
                       hotspots/
                         config.rb
                         analyzer/ # 
                   runner.rb # === generator :emit base used by each metric
                   formatter.rb # === generator :analyze / :to_h base used by each metric
                   environment.rb # contains info about ruby environs e.g. rails? mri? supports_ripper? 192?
                   file_system_operations.rb # ensure directories exists, knows code_dirs
                   report.rb # uses current results and historical
                     report/template.rb 
                     report/html_template.rb
                   graph
                     graph/grapher.rb
                   logger.rb
                   cli.rb
                     cli/etc.
                   parser.rb # has helpers that can get class and method name for a line number
                   run.rb / controller.rb # coordinates configuring, running, formatting, and outputting metrics
  tasks/
    metric_fu.rake
.metrics

Part of doing this is to not make the Configuration object create methods and instance variables everywhere. Each metric can configure itself and add itself to metric_fu (allows for plugins).

[robincurry]

Directory structure seems reasonable to me. I think a lot of the templating/graphing stuff could move underneath formatter (i.e. /lib/metric_fu/formatter/html/templates/ and /lib/metric_fu/formatter/html/graphs/). What do you think?

[bf4]

yes

On Sat, Jul 13, 2013 at 10:54 AM, Robin Curry notifications@github.comwrote:

Directory structure seems reasonable to me. I think a lot of the
templating/graphing stuff could move underneath formatter (i.e.
/lib/metric_fu/formatter/html/templates/ and
/lib/metric_fu/formatter/html/graphs/). What do you think?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/94#issuecomment-20921867
.

[bf4]

Looking good. Lemme know if you want to chat about this here, on skype, or elsewhere

[robincurry]

Cool, yeah. Anytime here, or I'm usually on gtalk (robin.curry@gmail.com) during the day and off and on in the evenings.

Now that the builds are passing, I think I've just about wrapped up what I had in mind on this pending any additional feedback you might have.

[bf4]

I honestly want to 1) not have long-running branches 2) release code right
now that either adds features, fixes bugs, or doesn't break anything... so,
merging is good.. did you update the docs / README?

[robincurry]

Ok, took a stab at updating README and HISTORY. Let me know if you need me to modify anything. Thanks @BJ4!

[bf4]

Try running with RUBY_OPTS=-w or see here for warnings to address

btw, you got my handle wrong above; I'm bf4 :)

I'm looking at the commits now

[bf4]

Please add comment what this method does

[robincurry]

Will do!

[bf4]

So, in YaRDoc format, I believe this would be return, see example

# Build a nicely formatted list of built-in
# formatter keys and their descriptions
# @see MetricFu::Formatter::BUILTIN_FORMATS
# @example "  yaml :  ['MetricFu::Formatter::YAML', 'Generates the raw output as yaml']"
# @return [Array<String>] in the form of 
#    "   <key>  : <description>."
def format_descriptions

[bf4]

Don't you want to use constantize here, now that you've introduced it?

BTW, I hope we can soon clean up how hard to read the code is in this method.

[robincurry]

Yes, indeed. Was just trying to limit changes to code outside of the realms of the PR.

[bf4]

I've made a bunch of comments. This is awesome! You now have commit access when you feel ready to merge it in.

[robincurry]

Ack! Sorry about the handle mistake. The perils of late night coding!

[bayleedev]

👍

[bf4]

Probably has:

require 'metric_fu/formatter/my_custom'

[bf4]

This is so awesome. Great work and great code!

[bf4]

Got an error running tests on cruby 2.0.0-p195 on mac osx leopard via rvm

Failures:

1) MetricFu::Formatter::HTML In general creates a report yaml file
 Failure/Error: expect {
   The file "tmp/metric_fu/report.yml" existed before, so this test doesn't work
 # ./spec/metric_fu/formatter/html_spec.rb:23:in `block (3 levels) in <top (required)>'

And I see RailsBestPractices now has active_support as a dependency. Ugh

[robincurry]

Hrm. Running the same locally and seems ok. As well as on travis. Happen to catch an rspec seed? Maybe order dependent? Something not getting cleaned up in a previous spec?

[bf4]

Dunno, didn't see a seed. Passed again for me. Ran with order='random', got failures with seeds: 61176, 18737

[robincurry]

Well, I've run the specs a good 20-30 times and can't seem to reproduce the spec failure. I'm going to go ahead and merge but I'll keep a close eye on it. I know there's nothing worse than an intermittent failing spec.

[bf4]

Yay!