psyho / bogus

Fake library for Ruby
Other
359 stars 14 forks source link

remove Gemfile.lock from Git #51

Closed ktdreyer closed 10 years ago

ktdreyer commented 10 years ago

It's tedious to remember to bump the bogus version here, and we shouldn't be storing this file in Git anyway.

chastell commented 10 years ago

I’m not sure what @psyho’s policies are, but a tentative :-1: from me – I’d rather have Gemfile.lock in the repo and make sure everyone develops with the same gem versions.

ktdreyer commented 10 years ago

I appreciate the quick feedback. Here's a couple of similar requests that were accepted:

Also see the blog article by Yehuda Katz: http://yehudakatz.com/2010/12/16/clarifying-the-roles-of-the-gemspec-and-gemfile/

chastell commented 10 years ago

I know all the reasons (but appreciate the links!) and I do agree with them, but I still believe there’s no harm in the dev team sharing the same environment. If a version of a dependency allowed by gemspec is problematic, that’s a bug regardless of what ends up in Gemfile.lock.

That said, I’m happy to follow the community practice and I’ll merge this one as soon as I figure out how to rerun Travis. :)

chastell commented 10 years ago

Ok, I figured out why this build blows up on Ruby 2 while it wasn’t with Gemfile.lock present (and it actually serves as a good example in favour of dropping the file) – without the Gemfile.lock one can run Bogus against activerecord 2.3 which blows up under Ruby 2.

Can you please adjust the dependency in bogus.gemspec like this?

s.add_development_dependency 'activerecord', '>= 3', '< 5'
ktdreyer commented 10 years ago

Looks like the Travis build is now failing because of https://github.com/nulldb/nulldb/pull/32

This is fixed in nulldb's Git master, but it's not fixed in any released version of the nulldb gem. Since I ran into this when I was packaging nulldb for Fedora, a couple weeks ago I requested that the nulldb folks release a formal update to rubygems, at https://github.com/nulldb/nulldb/issues/47 . No word so far.

chastell commented 10 years ago

Hmm, but why would it (a) work before and (b) work for me locally?

Wouldn’t pinning activerecord-nulldb-adapter to ~> 0.2.3 be enough (this is what was in Gemfile.lock and what I get locally when I bundle)?

ktdreyer commented 10 years ago

For the record, I've amended the prior commit "restrict activerecord to versions 3.x through 4.x" (https://github.com/ktdreyer/bogus/commit/63e94f6bd5dabe46fbc1b1b793efa6cb5daf30f2) to the new commit, "restrict ActiveRecord to versions 3.x " (https://github.com/ktdreyer/bogus/commit/9fda0adfda725190a6334e5dbc5452f1ab0892e3). GitHub doesn't do a great job tracking what happens during a rebase in a pull request.

With this adjustment, TravisCI builds now pass, so I think it's safe to merge this.

Regarding your question "why would it work before?" - The Gemfile.lock in Git forced activesupport/activerecord to version 3.2.13. This is a version that allows the activerecord-nulldb-adapter gem to work.

Regarding your question "why would it work for me locally?" - do you have activesupport/activerecord already locked to version 3.2 in your local Gemfile.lock? If so, the activerecord-nulldb-adapter gem will work when you run "bundle exec rake test". I suspect that if you rename your Gemfile.lock to something else, run "bundle install", and then run the tests, then the tests would fail without this pull request.

chastell commented 10 years ago

Ok, so the (second) issue wasn’t that activerecord-nulldb-adapter neccesarily needs a new release to work, but rather than its current release doesn’t work with activerecord 4. Thanks for the clarifications – and so much for your time!