Skip to content
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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Swimminschrage
Copy link

Adding git support to metric_fu to allow users to specify a git hash to run metric_fu against.

Saves the resulting yml file using the git hash as the filename. This can allow users to diff results across change lists.

@bf4
Copy link
Member

bf4 commented Dec 24, 2013

First off, I haven't looked at the code yet, but awesome for making this PR. Also, I want to reference related issues #96 and #107

s.cert_chain = ['certs/bf4.pem']
s.signing_key = File.expand_path("~/.ssh/gem-private_key.pem") if $0 =~ /gem\z/
#s.cert_chain = ['certs/bf4.pem']
#s.signing_key = File.expand_path("~/.ssh/gem-private_key.pem") if $0 =~ /gem\z/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing you didn't mean to include this in the PR, but I understand why you made it :)

@bf4 bf4 mentioned this pull request Dec 24, 2013
58 tasks
@Swimminschrage
Copy link
Author

I addressed many of the issues you brought up (thanks for the suggestions BTW). Let me know if there's anything else that I can take a look at.

I may still try to abstract out the version control system if possible.

@bf4
Copy link
Member

bf4 commented Jan 15, 2014

Please rebase against master, amend to remove extraneous commits, and get the tests passing!

@@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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

@bf4
Copy link
Member

bf4 commented Jan 15, 2014

Let me know if you want to pair on this. I have a lot more comments.

@bf4
Copy link
Member

bf4 commented Mar 24, 2014

I've been thinking.. this functionality should probably use the git tooling included in Churn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants