DruRly / gemrat

Save Time. Add the latest version of gems to your Gemfile from the command line.
http://bit.ly/18O9sNO
MIT License
170 stars 12 forks source link

more enhacements #12

Closed shime closed 11 years ago

shime commented 11 years ago

Main improvements:

I hope it's good to merge!

DruRly commented 11 years ago

Looks great so far! :metal: In the process of reviewing

mivanek commented 11 years ago

I'm a little late to the party, but here are some of the changes I've implemented in this pull request:

I'm currently working on implementing --environment flag so you can specify which environment you want to add the gem to. It should have support for multiple environments as well. I hope I'll be done some time in the next two days, maybe even today. Hope you're satisfied with the changes so far, if you have any comments, please let me now.

shime commented 11 years ago

:+1: :+1: :punch: :boom: :metal:

DruRly commented 11 years ago

I noticed that when detecting whether a gem already exists in the Gemfile, it does not recognize gems which do not have a version.

Steps to replicate:

  1. Start with a Gemfile that includes sidekiq without a version number.
  2. Run gemrat sidekiq
  3. You receive:

    => gem sidekiq, 2.12.4 added to your Gemfile.

    => Bunlding...

Version bumping and update confirmations work great!

This is still good to accept. Just noticed this and would like to get around to it at some point. Still testing. Thanks again guys!

mivanek commented 11 years ago

Thanks for the heads up. I was just talking to @shime earlier today and we realized that there are some edge cases/situations that we probably haven't covered too well, and I guess this is one of them. We've also realized that implementing the -e/--environment flag might take up more time than we anticipated because there are a couple of variations we need to cover that we haven't originally thought of, so it might take us a couple of days more than what I have anticipated earlier. Either way, we'll try to cover all that we can think of in the next pull request. If you think of or find any other bug/edge case that we maybe haven't noticed, please let us know. These things can easily go under the radar.

Cheers. :beer:

DruRly commented 11 years ago

Thanks for the update @mivanek. There's also a failing test which targets the -e/--environment flag functionality that you just mentioned. I don't think you intended for this to be in the pull. Would guys like for me to remove this and merge or edit the pull yourselves?

mivanek commented 11 years ago

You can remove the test and merge if you want to. I think I've also let a require 'pry' in gemrat.rb slip through, so you can remove that as well, if you haven't already.

DruRly commented 11 years ago

I changed the test to pending and merged. It's released as v0.4.0 and tagged. Added you to authors. Thanks again!

shime commented 11 years ago

:sunglasses: