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.

Sign up for GitHub

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

Jump to bottom

Add churn option (--ignore_files) #170

Closed
wants to merge 4 commits into from
Closed

Add churn option (--ignore_files) #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
88c358e
add ignore option
such Nov 7, 2013
97d06a3
add test churn ignore flag
such Nov 7, 2013
5e431f2
fix churn option parsing
such Nov 7, 2013
fde0282
add default churn option
such Nov 7, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/metric_fu/metrics/churn/churn.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,12 @@ def build_churn_options
def has_option?(churn_option)
options.include?(churn_option)
end

def churn_options
{
:minimum_churn_count => '--minimum_churn_count',
Copy link
Member

Choose a reason for hiding this comment

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

If you're up for it, this looks like a great opportunity to begin work on #167 which is basically to not require code changes for most of a metric's run options, but just to enable passing them through.

For example

:ignore_files => %q("Gemfile, Gemfile.lock, config/locales/fr.yml, config/locales/en.yml, db/schema.rb")

would map to the command-line argument

--ignore_files Gemfile,Gemfile.lock,config/locales/fr.yml,config/locales/en.yml,db/schema.rb

If you chose to do this, please create the options hash with the Map gem, instead of a the hash primitive. (It's included in metric_fu via churn)

:start_date => '--start_date'
:start_date => '--start_date',
:ignore_files => '--ignore_files'
}
end

Expand Down
2 changes: 1 addition & 1 deletion lib/metric_fu/metrics/churn/init.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def name
end

def default_run_options
{ :start_date => %q("1 year ago"), :minimum_churn_count => 10}
{ :start_date => %q("1 year ago"), :minimum_churn_count => 10, :ignore_files => "" }
Copy link
Member

Choose a reason for hiding this comment

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

see comments in churn generator

Copy link
Member

Choose a reason for hiding this comment

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

@such, looks like this causes spec failure at https://github.com/metricfu/metric_fu/blob/master/spec/metric_fu/configuration_spec.rb#L163

Mind updating the spec to reflect the additional option?

Copy link
Author

Choose a reason for hiding this comment

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

Actually I think we might want to close this pull request. I started working on #167 and it should be ready soon enough. What do you think ?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, sure, that would be great!

Copy link
Member

Choose a reason for hiding this comment

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

@such Please feel free to open a work-in-progress (wip) if you'd like some discussion before you're done, or write your thoughts on the google group. The two major issues, off the top of my head

  1. backwards compatibility
  2. metric-related options for metric_fu that aren't passed to the metric
  3. long vs. short option arguments having one dash or two :)

Copy link
Member

Choose a reason for hiding this comment

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

It might also be a good time to start a 'shared spec' so as not to duplicate tests. (and yes, I know that this is four, not two :)

end

def has_graph?
Expand Down
7 changes: 6 additions & 1 deletion spec/metric_fu/metrics/churn/churn_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@

it "initializes with given minimum_churn_count option" do
churn = MetricFu::ChurnGenerator.new( { :minimum_churn_count => 5 })
churn.send(:build_churn_options).should == "--yaml --minimum_churn_count=5"
churn.send(:build_churn_options).should == '--yaml --minimum_churn_count=5'
end

it "initializes with given ignore option" do
churn = MetricFu::ChurnGenerator.new({:ignore_files => %q("Gemfile, Gemfile.lock") })
churn.send(:build_churn_options).should == '--yaml --ignore_files="Gemfile, Gemfile.lock"'
end
end

Expand Down