Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to Ruby 2.2 and RuboCop 0.38 #371

Merged
merged 5 commits into from
Apr 22, 2016
Merged

Update to Ruby 2.2 and RuboCop 0.38 #371

merged 5 commits into from
Apr 22, 2016

Conversation

e2
Copy link
Contributor

@e2 e2 commented Apr 22, 2016

  • expect at least Ruby 2.2
  • fix lots of RuboCop offense

Reasons:

  1. Security - Ruby 2.0 is EOL, Ruby 2.1 is in "security-backports only" mode, Ruby 2.2 is recommended, upgrade path shouldn't be an issue.
  2. Dependency would be Ruby 2.3, except certain bugfixes are still unreleased
  3. More contributor friendly (better code, no restrictions on using more recent Ruby features)
  4. Moving focus away from spending time supporting "non-recent" versions of Ruby until Ruby 3.x to promote newer language features (instead of wasting the effort of core-devs to create a better language)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 97.704% when pulling e21066c on e2-mri2.2-rubocop_overhaul into 3dea1ba on master.

@e2 e2 merged commit b3899f7 into master Apr 22, 2016
@e2 e2 deleted the e2-mri2.2-rubocop_overhaul branch April 22, 2016 22:19
@parkr
Copy link

parkr commented Apr 23, 2016

😦 Getting lots of reports from Jekyll users that they're now having problems installing listen. What is the need for the Ruby 2.2 dependency?

@e2
Copy link
Contributor Author

e2 commented Apr 23, 2016

What is the need for the Ruby 2.2 dependency?

Ruby 2.0 is EOL.

Ruby 2.1 is already in security-backports only.

Upgrading to Ruby 2.2 is recommended: https://www.ruby-lang.org/en/news/2016/03/30/ruby-2-1-9-released/

I bumped the version minor to allow pinning to 3.0.x and for backports.

If there's anything reasonable I can do, let me know.

@e2
Copy link
Contributor Author

e2 commented Apr 23, 2016

Getting lots of reports from Jekyll users that they're now having problems installing listen

Can you give me an example?

@jmoody
Copy link

jmoody commented Apr 25, 2016

I would have appreciated if this change had been made in major release.

This change has been very disruptive.

@e2
Copy link
Contributor Author

e2 commented Apr 25, 2016

@jmoody - how so? I find it baffling that multiple people have called this disruptive without a single compelling example.

@parkr
Copy link

parkr commented Apr 25, 2016

@e2 You already saw jekyll/jekyll-watch#37 and jekyll/jekyll#4835. One issue is not a representation of all the problems we're getting – I get a number through Twitter and private channels like email.

The situation for the end user is this:

  1. Listen is a library, semantically-versioned.
  2. Jekyll depends on Listen as ~> 3.0, assuming 3.1, 3.2, etc will all be compatible.
  3. I'm a Jekyll user, happily using all of the available Ruby versions for Jekyll, which includes Ruby 2.1 (still supported!!)
  4. I go to upgrade Jekyll, and RubyGems picks up on a minor change to Listen which should be compatible, right?
  5. RubyGems refuses to install a minor bump of Listen because I'm using Ruby 2.1, which is still supported by Jekyll and by Ruby Core.

The argument is that semantic versioning requires compatibility not only in its public APIs, but also with the versions of Ruby it supports. We couldn't drop Ruby 1.9 support in Jekyll until Jekyll 3 – a major bump. We felt folks who had ~> 2.5 in their Gemfiles should not ever have problems upgrading until they manually changed it to ~> 3.0 at which time it would say they had to upgrade.

Breaking compatibility is for a major bump, not a minor one.

@e2
Copy link
Contributor Author

e2 commented Apr 25, 2016

One issue is not a representation of all the problems we're getting – I get a number through Twitter and private channels like email.

Please open up issues in the Listen tracker then. How can I properly address an issue if all I'm told is "everything is broken" with no examples?

I go to upgrade Jekyll, and RubyGems picks up on a minor change to Listen which should be compatible, right?

Read the changelog - they're ridiculously short. People shouldn't upgrade mindlessly. They don't upgrade Ruby mindlessly, right? I mean upgrading from 2.0 to 2.3 should be a breeze, right?

RubyGems refuses to install a minor bump of Listen because I'm using Ruby 2.1, which is still supported by Jekyll and by Ruby Core.

But no longer supported by Listen. De facto. So the fact this wasn't done earlier means it was a bug. (Bad dependency specified).

The argument is that semantic versioning requires compatibility not only in its public APIs, but also with the versions of Ruby it supports.

No, Ruby should be treated just like any other gem. If it doesn't follow semver, that's a Ruby problem, not a Listen problem.

Why can a user upgrade Listen and expect a minor bump to be flawless, while somehow I'm not allowed to make that exact same assumption with Ruby? How is upgrading Listen "easy" while upgrading Ruby somehow requires Listen to "be fixed"?

Ruby 2.3 "should be compatible" with Ruby 2.1, right? Same argument.

We couldn't drop Ruby 1.9 support in Jekyll until Jekyll 3 – a major bump.

Do you have a Jekyll version that's based on Ruby 2.2? One where I can submit PRs that work only on Ruby 2.2? Then that's a discrimination too! Discrimination against progress and benefits for both users and developers!

Will you release Jekyll 4 just for the sake of supporting Ruby 2.3 only? Probably not. Again, it's hypocrisy, because that's exactly what you're expecting me to do with Listen.

Breaking compatibility is for a major bump, not a minor one.

Please show me how I broke compatibility based on an API provided by Listen. I don't provide a Ruby API.

I'm not arguing for the sake of arguing. I'm baffled by a lack of actual use cases where this is a problem.

What if the problem isn't "in Listen"?

@e2
Copy link
Contributor Author

e2 commented Apr 25, 2016

@parkr - I think this nails it:

RubyGems refuses to install a minor bump of Listen because I'm using Ruby 2.1

So the user is choosing to upgrade Listen but not Ruby? Now the user is forcing me to either provide and maintain 2 codebases - or - stick to an outdated version of Ruby. So their demand for seamless "backward compatibility" is preventing my experience of seamless "forward-compatibility".

And by "outdated" I mean "outdated":

https://www.ruby-lang.org/en/news/2016/02/24/support-plan-of-ruby-2-0-0-and-2-1/

"This means that after the release of 2.1.9 we will never backport any bug fixes to 2.1 except security fixes."

So the user is attempting suicide and I'm support to support that? How is that ethical even? So if users expect me to murder my neighbors, I should comply because "users expect me to support their needs"?

@e2
Copy link
Contributor Author

e2 commented Apr 25, 2016

@parkr, @jmoody - README's update for both Guard and Listen. (details: Bundler prerelease solves the issue - it's awesome!).

@e2
Copy link
Contributor Author

e2 commented Apr 25, 2016

@parkr - consider unlocking the Listen gem dependency (depending on the status of Bundler prerelease version).

@rymai
Copy link
Member

rymai commented Apr 26, 2016

I just want to thank @e2 for pushing the adoption of Ruby 2.2+ forward!

I find it really good that installation fails, not usage.

What's even more awesome is that new Bundler will bring the best of both worlds: smooth installation and early dependency on new Ruby versions in .gemspec files!

@tfausak
Copy link

tfausak commented Apr 26, 2016

This change broke my build. See https://travis-ci.org/orgsync/stoplight/jobs/125913789. I don't even depend on listen.

In my opinion, changing the versions of Ruby that you support is a breaking change that should be made as part of a major version change.

@e2
Copy link
Contributor Author

e2 commented Apr 26, 2016

@tfausak - broken builds on Ruby 2.0 and 2.1. This works as expected, because both Ruby versions are no longer supported. Both contain lots of unfixed bugs.

Please remove building on Ruby 2.0 and 2.1 from Travis.

(Even if Listen "fixes" the problem, you're still using broken Ruby versions. I don't see why Listen should support that).

@tfausak
Copy link

tfausak commented Apr 26, 2016

Please don't tell me how to administer my project. Stoplight supports Ruby 2.0 and 2.1. I take support of Ruby versions seriously, as seen in AaronLasseigne/active_interaction#343.

Furthermore, Ruby 2.0 and 2.1 aren't "broken". Ruby 2.0 has reached end of life, so people probably shouldn't be using it. Ruby 2.1 is still receiving security fixes, so it is supported.

I want to take a second to address this comment, which you've made here and middleman/middleman#1891 (comment):

Ruby 2.3 "should be compatible" with Ruby 2.1, right?

Ruby doesn't follow Semantic Versioning. See https://www.ruby-lang.org/en/news/2013/12/21/ruby-version-policy-changes-with-2-1-0/:

MINOR: increased every christmas, may be API incompatible

@e2
Copy link
Contributor Author

e2 commented Apr 26, 2016

@tfausak - My first question: are you reasonable or are you pissed off?

Please don't tell me how to administer my project.

I'm not. I'm just making suggestions on what the fix should be.

Stoplight supports Ruby 2.0 and 2.1

The Ruby core team doesn't: https://www.ruby-lang.org/en/news/2016/03/30/ruby-2-1-9-released/

"After this release we will never backport any bug fixes to 2.1 except security fixes."

How is "not backporting any bug fixes" considered "support"?

Furthermore, Ruby 2.0 and 2.1 aren't "broken".

They are - in terms of security. Isn't security important for you? Check this CVE out: https://www.ruby-lang.org/en/news/2015/12/16/ruby-2-2-4-released/

And that's 2.2.4 (!) So by supporting even 2.2.3, you're supporting a security vulnerability.

Ruby doesn't follow Semantic Versioning.

In terms of C API, it doesn't true. But I'm not writing a Ruby extension here.

@e2
Copy link
Contributor Author

e2 commented Apr 26, 2016

@tfausak - also, how is not upgrading to Ruby 2.2 considered "important to support".

Also, a broken build is a good thing. Broken production code is not.

Listen <= 3.1.x is broken, because it advertises a compatibility which was:

  1. never official (compared to Readme, which is part of SemVer as documentation)
  2. never tested (Travis)

So bumping the dependency is actually a bugfix. I shouldn't even need a minor version bump.

@tfausak
Copy link

tfausak commented Apr 26, 2016

I am pissed off. You broke my build basically for no reason. But I am done dealing with this. I removed Guard (and therefore Listen) from the Ruby projects that I maintain.

@e2
Copy link
Contributor Author

e2 commented Apr 26, 2016

You broke my build basically for no reason.

For no reason apparent to you. It was a well considered trade-off.

But if you're pissed off, what's the use...

@e2
Copy link
Contributor Author

e2 commented Apr 26, 2016

I didn't actually break the build - it's just that Bundler at 1.11.x doesn't resolve based on Ruby version constraint. I didn't know that. All because it fails to install a working set of dependencies, doesn't mean one isn't possible. Bundler 1.12 fixes this.

@e2
Copy link
Contributor Author

e2 commented May 5, 2016

If Listen is only used for development, it shouldn't be installed on Travis at all

E.g. https://github.com/guard/listen/blob/03c7052/.travis.yml#L2

This means you can still use Listen in development, and yet avoid installing lots of unnecessary dependencies on Travis. (Which means: faster Travis builds).

FWIW, I compiled the discussions into: https://github.com/guard/listen/wiki/Support-for-older-versions-of-

joshpencheon added a commit to NHSDigital/ndr_support that referenced this pull request Jun 8, 2016
See (heated) discussion on: guard/listen#371

Bundler 1.12+ should be detecting Ruby version dependencies, but doesn't
seem to be...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants