search

ashkan18 / graphlient

Ruby GraphQL Client
MIT License
251 stars 44 forks source link

Drop Support for Ruby 2.2 and Lock RuboCop #45

Closed jonallured closed 6 years ago

jonallured commented 6 years ago

Hi Friends!

While updating Bearden to v0.3.1, I had to fix a test and I was confused in a couple ways, so I decided to fix those bits. I made this PR for you!! 🐻❤️🐻❤️🐻❤️

dblock commented 6 years ago

Thanks Jon, can you please fix the Rubocops too to get 💚 here? Lock the version of rubocop and rubocop -a ; rubocop --auto-gen-config. Thanks!

jonallured commented 6 years ago

Well, I expanded the scope of this PR after @dblock's suggestion. Looks like we didn't have RuboCop locked, which meant every build on Travis could and would use a different version - I've locked this down to the latest as of today.

As a part of complying with RuboCop 0.56, the safe navigator was used, which isn't available in Ruby 2.2, so I also dropped support for that version. I feel it's time as that version of Ruby is actually EOL at this point. I think this means that merging this PR would constitute a version bump, but I didn't take the step to make that change as I assume there's a release process that would do it.

yuki24 commented 6 years ago

@dblock @ashkan18 This is technically going to be a breaking change. We'd have to bump the major version if we wanted to follow semver strictly.

dblock commented 6 years ago

I think we messed up in the previous release and somehow merged broken code. The class name changed before, this is just fixing the test.

dblock commented 6 years ago

@jonallured If you want to challenge yourself to come up with a solution where danger-changelog can help you fix things, https://github.com/dblock/danger-changelog/issues/33

dblock commented 6 years ago

Seems like we're hitting https://github.com/travis-ci/travis-ci/issues/8978 on newer rubies. Try adding 

before_install:
  - gem update --system

To .travis.yml?

jonallured commented 6 years ago

Ha! I've been on an upgrade rampage today and have hit this a few times already but didn't know it was a tracked issue. I think 90582e1 should do it, but I've also had to do a bundle install to get the latest of that as well. We shall see!

dblock commented 6 years ago

Lol danger still not happy with you ;) Put back * Your contribution here. into CHANGELOG

jonallured commented 6 years ago

I've got a good feeling about this next one!! 🤞

dblock commented 6 years ago

Merged

dblock commented 6 years ago

@jonallured Maybe take care of https://github.com/ashkan18/graphlient/issues/44?