adarshpandit/gitsucker — pull request #1 by gabebw (2012-05-25 03:09:00+00:00)
Title: Fixups.

Quick overview of what I did:

* Split functionality across some new classes
* Added tests
* Rakefile
* Gemfile
* everything's now in the `Gitsucker` module
* More of a Rubygem structure (e.g. `lib/gitsucker.rb` is the only thing you have to require, command-line script is in `bin`)
* `Author#stats` has been replaced by accessor methods. The Author shouldn't know what the Table needs for its formatting, it should just make its info available and let the Table take what it needs.]

Points that might be helpful for you to check out in the future:
* Use a [StringIO](http://www.ruby-doc.org/stdlib-1.9.3/libdoc/stringio/rdoc/StringIO.html) from Ruby's standard library as an output source so you can test output.
* ShamRack is pretty friggin' amazing. Its `https` handling is sort of odd - instead of `ShamRack.at('https://foo.com')` you have to do `ShamRack.at('foo.com', 443)` since HTTPS operates on port 443. But it's a great little library.

Random thoughts that could improve this:

* Have an `AuthorPrinter` that does what `Table#display_author_stats` does now, probably it would take an author in its constructor. The `AuthorPrinter` has the advantage of a public method (probably `AuthorPrinter#print`) that you can test, instead of the private `display_author_stats` method.
* The `rescue` in Table is unfortunate - using exceptions for control flow is Bad. Instead, I'd use a `RepoExistenceChecker` or something and have it return true/false depending on if the repo exists. That way you have an if/else instead of a begin/rescue.
* I'm undecided on whether actually hitting Github for Cucumber tests is a good idea. I'm leaning towards "Yes, it is a good idea", because it's a tiny test and takes like 3 seconds, plus it's an actual real-life integration test.

If you have any questions about or criticisms of any of the code, don't hesitate. Follow your heart.