search

haf / semver

gem: "semver2" fork. simple command line util & ruby module for managing versions according to http://semver.org
MIT License
35 stars 10 forks source link

Introducing XSemVer::Runner #3

Closed jameschildress closed 11 years ago

jameschildress commented 11 years ago

Refactoring

Hello again! I wanted to update the bin/semverconsole tool to use the features introduced in my previous pull request. Unfortunately, that file was basically one long, untested case statement. To simplify this and future contributions, I refactored most of the bin/semver code into the XSemVer::Runner class, which uses methods and aliases instead of a case statement to handle branching code.

Testing

I've added a comprehensive set of specs to spec/runner_spec.rb which tests every command that can be run from the console.

New Commands

After the existing commands were tested, I added two new commands to the console tool:

The README.md file and semver help command have been updated to reference the new commands. The version of this gem has been incremented to 3.3.0.

To-Do

I stubbed the SemVer::FILE_NAME constant in spec/runner_spec.rb. This will cause the spec/semver_spec.rb specs to fail if both files are run together using the rspec spec/ command. For the tests to pass, both files must be run separately.

To fix this, I intend to move SemVer::FILE_NAME from a constant into a class method, so that it can be stubbed on a per-test basis without breaking other tests.

haf commented 11 years ago

Hello,

I've started reading the code and it's much more ideomatic ruby than a case statement, so I like it. Catching NoMethodError is a nice way to test for command validity in a dynamic OOP/message language.

In fact, consistently giving awesome pull requests gives me a reason to give you commit access.

In terms of this pull request, I was thinking you might want to do a feature-branch that is mergeable to master (for documentation purposes/history). Also, the TODO you mention would be a good addition to the mix; are you considering StringIO in the tests (http://stackoverflow.com/questions/1484129/ruby-send-logger-messages-to-a-string-variable)?

jameschildress commented 11 years ago

I'm honored! I've just pushed one additional commit which adds the SemVer.file_name method which returns the value of FILE_NAME, and updated the specs accordingly. Now, all the specs can be run with rspec spec/ without any tests failing.

I'll get to merging, then. And yes, StringIO is exactly how I plan to improve the XSemVer::Runner tests.