-
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
Adding support for running metrics against a particular git hash #188
base: main
Are you sure you want to change the base?
Changes from all commits
5aee045
223780e
419a6e4
97d8552
02822a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ def initialize | |
def run(options={}) | ||
configure_run(options) | ||
measure | ||
reset_git(options) if git_configured? options | ||
display_results if options[:open] | ||
end | ||
|
||
|
@@ -28,6 +29,7 @@ def display_results | |
private | ||
def configure_run(options) | ||
disable_metrics(options) | ||
configure_git(options) | ||
configure_formatters(options) | ||
end | ||
def disable_metrics(options) | ||
|
@@ -55,20 +57,64 @@ def configure_metric(metric, metric_options) | |
end | ||
end | ||
def configure_formatters(options) | ||
filename = options[:githash] # Set the filename for the output; nil is a valid value | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a little misleading, no? Why not just do metric_filename = options.fetch(:githash) { filename_timestamp(Time.now) }
def filename_timestamp(time)
time.strftime("%Y%m%d")
end |
||
|
||
# Configure from command line if any. | ||
if options[:format] | ||
MetricFu.configuration.formatters.clear # Command-line format takes precedence. | ||
Array(options[:format]).each do |format, o| | ||
MetricFu.configuration.configure_formatter(format, o) | ||
MetricFu.configuration.configure_formatter(format, o, filename) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need to pass the filename if we combine its logic with the default filename (filenamed_timestamp) |
||
end | ||
end | ||
# If no formatters specified, use defaults. | ||
if MetricFu.configuration.formatters.empty? | ||
Array(MetricFu::Formatter::DEFAULT).each do |format, o| | ||
MetricFu.configuration.configure_formatter(format, o) | ||
MetricFu.configuration.configure_formatter(format, o, filename) | ||
end | ||
end | ||
end | ||
def configure_git(options) | ||
return unless git_configured? options | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here you'd want to use a version-control adapter that can require and use arbitrary library code. See comment below re: gem dependency |
||
execute_with_git do | ||
begin | ||
@git ||= Git.init | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'd need an adapter with a consistent interface for git operations so that someone could, say, use the rugged gem and not need to change the code. |
||
@orig_branch ||= @git.current_branch | ||
|
||
mf_log "CHECKING OUT '#{options[:githash]}'" | ||
@git.checkout(options[:githash]) | ||
rescue Git::GitExecuteError => e | ||
mf_log "Unable to checkout githash: #{options[:githash]}" | ||
raise "Unable to checkout githash: #{options[:githash]}: #{e}" | ||
end | ||
end | ||
end | ||
def reset_git(options) | ||
return unless git_configured? options | ||
|
||
execute_with_git do | ||
begin | ||
@git ||= Git.init | ||
mf_log "CHECKING OUT ORIGINAL BRANCH '#{@orig_branch}'" | ||
@git.checkout(@orig_branch) | ||
rescue Git::GitExecuteError => e | ||
mf_log "Unable to reset git status to branch '#{@orig_branch}'" | ||
raise "Unable to reset git status to branch '#{@orig_branch}'" | ||
end | ||
end | ||
end | ||
def execute_with_git(&block) | ||
begin | ||
require 'git' | ||
yield | ||
rescue LoadError => e | ||
mf_log 'Cannot select git hash; please install git gem' | ||
raise 'Cannot select git hash; please install git gem' | ||
end | ||
end | ||
def git_configured?(options) | ||
options.size > 0 && options.has_key?(:githash) | ||
end | ||
def reporter | ||
MetricFu::Reporter.new(MetricFu.configuration.formatters) | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
require 'construct' | ||
require 'json' | ||
require 'pry-nav' | ||
require 'git' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is unnecessary if we use execute_with_git or something like that, no? |
||
|
||
# add lib to the load path just like rubygems does | ||
$:.unshift File.expand_path("../../lib", __FILE__) | ||
|
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.
If you feel ambitious, this would be a good place to factor out the notion of "Report Time" (what time in the code the reported metrics were run).
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 also seems mis-named. It's the filename, not the prefix.