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

set_code_dirs check for existence #129

Merged
merged 3 commits into from
Sep 25, 2013
Merged

set_code_dirs check for existence #129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e478e9
Instead of checking if it is a rails project, just include directorie…
Aug 21, 2013
9cdc96e
Fixing unit tests
Aug 21, 2013
d0fb778
Updating history.
Aug 21, 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
1 change: 1 addition & 0 deletions AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ Richard Huang
Dan Mayer
Benjamin Fleischer
Robin Curry
George Erickson
Copy link
Member

Choose a reason for hiding this comment

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

So, this is something I'm still thinking about, what should go in AUTHORS.

The AUTHORS file is used to set the authors for the gem. Should the authors be the maintainers, the people with significant contributions, or everyone? A common alternative is to have a CONTRIBUTORS file that lists everyone. I didn't create that when I added the AUTHORS file since I wasn't sure how I wanted to handle duplication. Thoughts @robincurry @danmayer @MGotink ? I could also ask @jscruggs how he decided to add people as authors of the gem.

Copy link
Author

Choose a reason for hiding this comment

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

Yea, I was unsure too, since I've done > 0.1% of the code.

Could always use git summary from https://github.com/visionmedia/git-extras.

Copy link
Member

Choose a reason for hiding this comment

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

You definitely get points for noticing it was there and editing it. :)

Copy link
Author

Choose a reason for hiding this comment

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

 project  : metric_fu
 repo age : 5 years ago
 commits  : 1152
 active   : 306 days
 files    : 220
 authors  : 
   456  Benjamin Fleischer      39.6%
   225  Jake Scruggs            19.5%
    73  Petrik                  6.3%
    64  Robin Curry             5.6%
    45  Dan Mayer               3.9%
    36  Édouard Brière        3.1%
    20  Grant McInnes           1.7%
    18  Richard Huang           1.6%
    18  jakescruggs             1.6%
    17  Andre Arko              1.5%
    15  Michael Stark           1.3%
    14  jscruggs                1.2%
    13  dan sinclair            1.1%
    11  Carl Youngblood         1.0%
     9  Avdi Grimm              0.8%
     7  Chris Griego            0.6%
     7  Jay Zeschin             0.6%
     7  Sean Soper              0.6%
     7  Sathish                 0.6%
     6  GeorgeErickson          0.5%
     6  David Chelimsky         0.5%
     5  Nick Quaranto           0.4%
     5  Andrew Timberlake       0.4%
     5  Andrew Selder           0.4%
     4  Mark Wilden             0.3%
     4  Martin Gotink           0.3%
     4  David Barri             0.3%
     4  Delwyn de Villiers      0.3%
     4  calveto                 0.3%
     4  Chris Mason             0.3%
     3  joshuacronemeyer        0.3%
     3  jayzes                  0.3%
     3  Guilherme Souza         0.3%
     3  Stefan Huber            0.3%
     2  unknown                 0.2%
     2  Extrovert               0.2%
     1  Paul Elliott            0.1%
     1  Alex Rothenberg         0.1%
     1  Andy Gregorowicz        0.1%
     1  Beau Fabry              0.1%
     1  Diego Carrion           0.1%
     1  Eric Wollesen           0.1%
     1  Hans Hasselberg         0.1%
     1  Jinzhu                  0.1%
     1  Joel Nimety             0.1%
     1  KAKUTANI Shintaro       0.1%
     1  Kevin Rutherford        0.1%
     1  Lars E. Hoeg            0.1%
     1  Matthew Gordon          0.1%
     1  Matthew Van Horn        0.1%
     1  Adam Bair               0.1%
     1  Randy Souza             0.1%
     1  Scyllinice              0.1%
     1  Tarsoly András        0.1%
     1  Todd A. Jacobs          0.1%
     1  benlovell               0.1%
     1  factorylabs             0.1%
     1  iain                    0.1%
     1  khall                   0.1%

Copy link
Author

Choose a reason for hiding this comment

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

Maybe 1% is a good cutoff :)

1 change: 1 addition & 0 deletions HISTORY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ As such, a _Feature_ would map to either major or minor. A _bug fix_ to a patch.
* Features
* Fixes
* Misc
* Check for app/lib existance as opposed to using rails? (George Erickson)
Copy link
Member

Choose a reason for hiding this comment

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

Should probably rewrite to something like 'Identify directories with code to analyze by checking if they exist. (No longer use :rails? as a proxy for checking if we should run on 'app'). (George Erickson, #129)

Copy link
Author

Choose a reason for hiding this comment

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

Will do


### MetricFu 4.4.0 / 2013-08-15

Expand Down
8 changes: 2 additions & 6 deletions lib/metric_fu/io.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,8 @@ def create_directories(*dirs)

# Add the 'app' directory if we're running within rails.
def set_code_dirs(config)
# TODO: Rather than check if we're running against a rails app,
# shouldn't we just check if the directories exist?
if config.rails?
@directories['code_dirs'] = %w(app lib)
else
@directories['code_dirs'] = %w(lib)
@directories['code_dirs'] = %w(app lib).select do |dir|
Dir.exists? dir
end
end

Expand Down
1 change: 1 addition & 0 deletions spec/metric_fu/configuration_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ def load_metric(metric)
describe 'if #rails? is true ' do

before(:each) do
Dir.stub(:exists?).and_return(true)
Copy link
Member

Choose a reason for hiding this comment

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

Is there, perhaps, a better way to do this? Maybe temporarily set the code_dirs yourself? Something less library-level global

BTW, I noticed that a number of tests fail when I touch config/environment.rb as our tests apparently don't mock the code_dirs properly somewhere. I also don't see anything terrible that happens when running rbp on a non-rails project... but that's off topic.

Copy link
Author

Choose a reason for hiding this comment

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

I should probably be less heavy handed with this stub. And only return fake the existence of an app directory.

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 think you can, that'd be great. Side-effects of one test leaking into another are hard to find and fix. (And I know there's already stuff like this in the test suite, but I want to remove that, too!)

@config = MetricFu.configuration
@config.stub(:rails?).and_return(true)
@config.reset
Expand Down