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

Fix Ruby 2.7 keyword args deprecation in DSL #1176

Merged
merged 2 commits into from Apr 8, 2020

Conversation

@JonRowe
Copy link
Member

JonRowe commented Apr 5, 2020

Fixes #1150, replaces #1154

@JonRowe JonRowe force-pushed the fix-kw-args-deprecation-in-dsl branch 2 times, most recently from 0f7d1d9 to 1ed0a75 Apr 5, 2020
@JonRowe JonRowe requested a review from pirj Apr 5, 2020
@pirj
Copy link
Member

pirj commented Apr 5, 2020

This change doesn't seem to fix the original issue:

    RSpec::Matchers.define(:be_foo) do |bar: nil|
      match { expect(true).to be true }
    end

    it 'works' do
      expect(:foo).to be_foo(bar: :qux)
    end
@JonRowe
Copy link
Member Author

JonRowe commented Apr 5, 2020

Oh hmph, then the original implementation can't have fixed it either.

@JonRowe JonRowe requested a review from benoittgt Apr 5, 2020
@JonRowe JonRowe force-pushed the fix-kw-args-deprecation-in-dsl branch from 1ed0a75 to 94c1d76 Apr 8, 2020
@JonRowe
Copy link
Member Author

JonRowe commented Apr 8, 2020

Added additional specs and fix.

Copy link
Member

benoittgt left a comment

LGTM Jon, but I have one question.

spec/rspec/matchers/dsl_spec.rb Show resolved Hide resolved
@JonRowe JonRowe force-pushed the fix-kw-args-deprecation-in-dsl branch from 94c1d76 to d25c75c Apr 8, 2020
@JonRowe
Copy link
Member Author

JonRowe commented Apr 8, 2020

Happy with this @pirj?

@pirj
pirj approved these changes Apr 8, 2020
Copy link
Member

pirj left a comment

No warnings! 👍 👏

@JonRowe JonRowe merged commit e48f774 into master Apr 8, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@JonRowe JonRowe deleted the fix-kw-args-deprecation-in-dsl branch Apr 8, 2020
@lostie
Copy link

lostie commented May 7, 2020

Hi there 👋 , when are you planning to release a new version of the gem that includes these changes?

@JonRowe
Copy link
Member Author

JonRowe commented May 7, 2020

As a volunteer project we don't have a timescale sorry. Soon, theres some other keyword warning removals in progress.

@pirj
Copy link
Member

pirj commented May 7, 2020

@lostie It would be very helpful if you could run RSpec's master against your project and open a ticket if you happen to experience more of those warnings.

@marcandre
Copy link
Contributor

marcandre commented May 7, 2020

@JonRowe a release would be appreciated, even if it doesn't resolve all issues, it resolves some of them. IMO, the time to create a release is so small compared to just about anything involving maintenance of a gem... when you have a great tests suite like rspec does, frequent bug fix releases make a lot of sense to me.

@marcandre
Copy link
Contributor

marcandre commented May 7, 2020

FWIW, this resolves the only warnings I had in 2.7 🎉

I had to modify my Gemfile with:

gem 'rspec', git: 'https://github.com/rspec/rspec.git'
gem 'rspec-expectations', git: 'https://github.com/rspec/rspec-expectations.git'
gem 'rspec-core', git: 'https://github.com/rspec/rspec-core.git'
gem 'rspec-mocks', git: 'https://github.com/rspec/rspec-mocks.git'
gem 'rspec-support', git: 'https://github.com/rspec/rspec-support.git'

I'm betting 0.25$ that figuring this out took me more than it takes to make a release.

@JonRowe
Copy link
Member Author

JonRowe commented May 7, 2020

@marcandre yes I plan to do one soon, the pattern is generally to try to batch releases together but not wait too long, after all too frequent releases are exhausting for both user and maintainer.

However I haven't had any "not tired" time since this was re highlighted and I don't do releases when I'm tired because I tend to make mistakes that are then very hard to reverse in a project as widely used as RSpec.

FWIW theres a bunch that goes into releasing one of the RSpec gems, checking any dependencies on the other gems, going through master and checking its been properly merged into 3-9-maintenance, then finally bumping the release, pushing it up, waiting for the build, then cutting the release is the final step.

JonRowe added a commit that referenced this pull request May 8, 2020
Fix Ruby 2.7 keyword args deprecation in DSL
JonRowe added a commit that referenced this pull request May 8, 2020
JonRowe added a commit that referenced this pull request May 8, 2020
@JonRowe
Copy link
Member Author

JonRowe commented May 8, 2020

This has been released as 3.9.2, I apologise for the delay but for your, 25¢ it took a good 45 minutes to check and sort the status of the two branches, get everything built and released, so I really hope it took you less time to figure out how to use the master branches 😂

@marcandre
Copy link
Contributor

marcandre commented May 8, 2020

Thanks @JonRowe I owe you 25¢ 😆
I'll make that a beer if we get the chance to cross paths!

@lostie
Copy link

lostie commented May 11, 2020

Thank you @JonRowe !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.