rspec / rspec-support

Common code needed by the other RSpec gems. Not intended for direct use.
https://rspec.info
MIT License
98 stars 103 forks source link

Drop Ruby < 2.3 support #436

Closed pirj closed 3 years ago

pirj commented 3 years ago

Better viewed ignoring indentation changes https://github.com/rspec/rspec-support/pull/436/files?diff=split&w=1

As per https://github.com/rspec/rspec/issues/61#issuecomment-687290045

Sibling PRs:

Minimal changes to rspec-core and rspec-mocks were required to run rspec-support's own spec suite, pushed to branches with the same name.

What has been done

Done:

pirj commented 3 years ago

For some reason, builds stopped triggering. Going to switch to "ready for review" temporarily just to figure out if it's the case (quite unlikely though).

benoittgt commented 3 years ago

For the build issue it is because of travisci config. https://github.com/rspec/rspec-support/pull/436/commits/112973040cbab3bfb69b23bc21b03bc084c93e00

pirj commented 3 years ago

@JonRowe Thanks for the review.

I've outlined the things to discuss in the description, including:

[?] extract rubocop changes into a separate pull request

I'll extract this into a separate follow-up pull request to 4-0-dev.

pirj commented 3 years ago

@JonRowe @benoittgt Appreciate if you can take a look at Discuss section of the description, there are some important questions, let me cite below:

  1. introduce https://github.com/ruby/ruby2_keywords dependency and remove respond_to?(:ruby2_keywords, true) check
  2. cleanup of build functions/predicates - any objections? (for sure in a separately pull request, and coming from rspec-dev)
  3. drop Ruby 2.3 - problematic'ish due to a dependency on outdated OpenSSL, EOL'ed, not supported by RuboCop, still used by 8% of users
  4. tweak clone_repo to accept a branch, use maintenance-branch's branch for main repos, and 4-0-maintenance for cloning rspec-rails since it's alienated anyway?
JonRowe commented 3 years ago

[?] introduce https://github.com/ruby/ruby2_keywords dependency and remove respond_to?(:ruby2_keywords, true) check

No, we can't introduce dependencies.

[?] drop Ruby 2.3 - problematic'ish due to a dependency on outdated OpenSSL, EOL'ed, not supported by RuboCop, still used by 8% of users

What Rubocop supports does not drive us. 2.3 is not really problematic.

[?] tweak clone_repo to accept a branch, use maintenance-branch's branch for main repos, and 4-0-maintenance for cloning rspec-rails since it's alienated anyway?

It already does this? Its just hardcoded based on the branch?

[?] extract rubocop changes into a separate pull request

Just remove them, they're unneeded.

pirj commented 3 years ago

and 4-0-maintenance for cloning rspec-rails since it's alienated anyway?

It already does this? Its just hardcoded based on the branch?

Doesn't seem to. The build was failing without drop-old-rubies rspec-rails branch. Also, it failed with default version.rb:

Resolving dependencies...

Bundler could not find compatible versions for gem "rspec-core":

  In Gemfile:
    rspec-core
    rspec-rails was resolved to 4.1.0.pre, which depends on
      rspec-core (= 3.11.0.pre)

Changed it to 4.0.0.pre to trick this build.

pirj commented 3 years ago

I'm getting stuck with removing WithKeywordsWhenNeeded and dealing with kwargs delegation related failures in combination with dropping support for old rubies. Removal of WithKeywordsWhenNeeded is tangential to the removal of rubies, and I'm going to remove all eval hacks, but not kwargs delegation hacks. I intend to address the removal of WithKeywordsWhenNeeded and kwargs delegation issues and hacks in a separate series of pull requests.

pirj commented 3 years ago

@JonRowe I intend to add back a jruby-9.1 build job, ~and make an attempt to remove ripper_supported? after that~.

pirj commented 3 years ago

No CI-related files included in this change! Suddenly all-green CI πŸŽ‰

@benoittgt @JonRowe welcome for a review.

PS and huge thanks for your work on JRuby/Windows/CI etc, guys. Your work has made this cleanup possible ❀️

pirj commented 3 years ago

Updates:

pirj commented 3 years ago

Update: combined ReentrantMutex with Mutex.

pirj commented 3 years ago

Update: reset the temporary maintenance-branch commit.

Green! πŸ’š

pirj commented 3 years ago

πŸŽ‰